Skip to content

Fix window title stuck on last opened webview#39259

Merged
Taym95 merged 1 commit intoservo:mainfrom
Taym95:fix-window-title-stuck-on-last-opened-webview
Oct 23, 2025
Merged

Fix window title stuck on last opened webview#39259
Taym95 merged 1 commit intoservo:mainfrom
Taym95:fix-window-title-stuck-on-last-opened-webview

Conversation

@Taym95
Copy link
Copy Markdown
Member

@Taym95 Taym95 commented Sep 11, 2025

Testing: tested manually switching between tabs and check if title changes.
Fixes: #39256

@Taym95 Taym95 requested a review from atbrakhi as a code owner September 11, 2025 13:00
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Sep 11, 2025
@Taym95 Taym95 force-pushed the fix-window-title-stuck-on-last-opened-webview branch from a10725f to 90427fa Compare September 11, 2025 13:00
Taym95
Taym95 commented Sep 11, 2025
View reviewed changes
yezhizhen
yezhizhen reviewed Sep 11, 2025
View reviewed changes
mrobinson
mrobinson requested changes Sep 11, 2025
View reviewed changes
@Taym95
Copy link
Copy Markdown
Member Author

Taym95 commented Oct 6, 2025

@mrobinson thanks for the review. How should we move forward with this PR, should we close it? I tried to look, but this seems like the only solution possible right now.

@mrobinson
Copy link
Copy Markdown
Member

mrobinson commented Oct 6, 2025

@mrobinson thanks for the review. How should we move forward with this PR, should we close it? I tried to look, but this seems like the only solution possible right now.

How would you feel about making these changes?

  1. Modify update_webview_data to take the window: Rc<dyn WindowPortsMethod> as an argument. It is available at the callsite.
  2. Modify WindowPortsMethods::set_title to be called WindowPortsMethods::set_title_if_changed.
  • This would still do nothing for headless window.
  • For headed windows this would compare the result of calling Window::title with WebView::page_title and only set the title if changed. It would return true or false depending on if a change occurred.
  1. update_webview_data would call this modified method.

@Taym95 Taym95 force-pushed the fix-window-title-stuck-on-last-opened-webview branch from 013ff8b to 7b0d1e8 Compare October 7, 2025 15:40
@Taym95
Copy link
Copy Markdown
Member Author

Taym95 commented Oct 7, 2025

@mrobinson thanks for the review. How should we move forward with this PR, should we close it? I tried to look, but this seems like the only solution possible right now.

How would you feel about making these changes?

  1. Modify update_webview_data to take the window: Rc<dyn WindowPortsMethod> as an argument. It is available at the callsite.
  2. Modify WindowPortsMethods::set_title to be called WindowPortsMethods::set_title_if_changed.
  • This would still do nothing for headless window.
  • For headed windows this would compare the result of calling Window::title with WebView::page_title and only set the title if changed. It would return true or false depending on if a change occurred.
  1. update_webview_data would call this modified method.

Thanks, I addressed this.

@Taym95 Taym95 force-pushed the fix-window-title-stuck-on-last-opened-webview branch 2 times, most recently from 20b7a17 to 1b72a4c Compare October 7, 2025 15:50
mrobinson
mrobinson requested changes Oct 8, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Thanks. just a few nits:

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 8, 2025
@Taym95 Taym95 force-pushed the fix-window-title-stuck-on-last-opened-webview branch from 1b72a4c to a5e0426 Compare October 19, 2025 19:29
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Oct 19, 2025
@Taym95 Taym95 force-pushed the fix-window-title-stuck-on-last-opened-webview branch from a5e0426 to 3b05efe Compare October 19, 2025 20:51
@Taym95
Copy link
Copy Markdown
Member Author

Taym95 commented Oct 23, 2025

Looking good. Thanks. just a few nits:

@mrobinson I addressed the comments.

mrobinson
mrobinson approved these changes Oct 23, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, I had forgotten this one. Just a few nits before landing.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Oct 23, 2025
@Taym95 Taym95 force-pushed the fix-window-title-stuck-on-last-opened-webview branch from 3b05efe to 6ec52d9 Compare October 23, 2025 15:05
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 23, 2025
@Taym95 Taym95 enabled auto-merge October 23, 2025 15:30
@Taym95 Taym95 added this pull request to the merge queue Oct 23, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 23, 2025
Merged via the queue into servo:main with commit 1ad3f06 Oct 23, 2025
33 checks passed
@Taym95 Taym95 deleted the fix-window-title-stuck-on-last-opened-webview branch October 23, 2025 17:02
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gterzian gterzian gterzian left review comments

@yezhizhen yezhizhen yezhizhen left review comments

@mrobinson mrobinson mrobinson approved these changes

@atbrakhi atbrakhi Awaiting requested review from atbrakhi atbrakhi is a code owner

Assignees

No one assigned

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

minibrowser: window title stuck on last opened webview

5 participants

@Taym95 @mrobinson @gterzian @yezhizhen @servo-highfive