Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

URL bar updating improvements #8

Merged
merged 5 commits into from
Dec 8, 2015
Merged

Conversation

diracdeltas
Copy link
Member

Update the url bar when changing active tabs or doing top-level redirects.

@bbondy to review

Update the URL bar at load start instead of load end since that is
what most browsers do.
@@ -63,6 +63,13 @@ class UrlBar extends ImmutableComponent {
AppActions.setNavBarFocused(true)
}

componentWillReceiveProps (newProps) {
let location = newProps.activeFrameProps.get('location') // TODO: update this during redirects
Copy link
Member Author

Choose a reason for hiding this comment

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

the location prop on the frame isn't getting updated during redirects. help fixing this would be appreciated.

test case:

  1. open a new tab
  2. load google.com, which redirects to https://www.google.com/foobar. URL bar is updated correctly to https://www.google.com/foobar.
  3. switch to another tab
  4. switch back to the original tab. the URL bar displays the original URL ('http://google.com') instead of the actual URL in the frame.

@bbondy
Copy link
Member

bbondy commented Dec 5, 2015

Looking good, hope the comments help. Haven't tried them but I think that's how to fix.

@diracdeltas
Copy link
Member Author

Thanks for the detailed comments and appstate documentation! I've updated the pull request to use the frame location prop.

This doesn't seem to be working on <meta> redirects though (ex: http://zyan.scripts.mit.edu/) because the webview redirect event isn't fired. Filed upstream electron/electron#3723

@bbondy
Copy link
Member

bbondy commented Dec 8, 2015

Looks great, nice work!
You may want to mirror that meta redirect issue on our side and reference the other issue if you think we should be handling on our own side and not forgetting about it for or around MVP. I'll merge this in.

bbondy added a commit that referenced this pull request Dec 8, 2015
@bbondy bbondy merged commit 17d7177 into master Dec 8, 2015
@diracdeltas
Copy link
Member Author

Fixed the redirect issue in a1eb2fb

@bbondy bbondy deleted the feature/update-urlbar-with-tab branch December 19, 2015 19:14
@bridiver bridiver mentioned this pull request Feb 11, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants