Update tab title on back/forward fixes #3200 #3202
Conversation
|
@bridiver pls review |
|
|
||
| var data = { | ||
| "title": document.title, | ||
| "url": window.location.href |
There was a problem hiding this comment.
do we need the url here? The title is already set to the url if page-title-updated isn't fired. I can't articulate a particular threat, but I just feel like it would be generally safer to use the url from frames.js rather than sending back window.location.href @diracdeltas ?
|
Thanks! This is definitely a reasonable change from the browser side and I think it's a good workaround, but I think we really want to retrieve the title from the navigation controller entry (not currently exposed in electron). I can open a separate ticket to make that electron change and we can merge this for now after comments. |
|
@bridiver Just simplified the tests. I have also fixed the ipc event to take the url from the frame if no title is provided. Let me know if there is something else 😄 |
|
actually I was wrong, @bsclifton added the method that is required for this in brave/muon@11b8318 |
|
@bridiver Thanks, I was looking for this in official electron docs, obviously I should have looked in another place 😄 I have changed the way I set the title, since now we don't get it from the web view, do you think I should get rid of |
|
I think you can remove |
|
@bridiver PR is updated. Let me know if all ok. Thank you very much for your feedback :) |
| @@ -773,6 +774,10 @@ class Frame extends ImmutableComponent { | |||
| this.webview.focus() | |||
| } | |||
| windowActions.setNavigated(e.url, this.props.frameKey, false) | |||
| // After navigating to the URL, set correct frame title | |||
| let webContents = this.webview.getWebContents() | |||
| let title = webContents.getTitleAtIndex(webContents.getCurrentEntryIndex()) | |||
There was a problem hiding this comment.
@bsclifton - if you add these methods to https://github.com/brave/electron/blob/master/lib/renderer/web-view/web-view.js#L391 they can be called directly on the webview
|
++ Thanks! |
git rebase -ito squash commits if needed.The
page-title-updatedevent on webview is not triggered when invoking back/forward actions. This PR adds ipc message that makes sure window data such as title is always sent back to electron, so it can be properly updated when the page changes.Reviewers: @bbondy @bridiver @bsclifton @diracdeltas