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

Back button is enabled after page load on /firstrun flow #3296

Closed
shane-tomlinson opened this issue Nov 18, 2015 · 6 comments · Fixed by #3395
Closed

Back button is enabled after page load on /firstrun flow #3296

shane-tomlinson opened this issue Nov 18, 2015 · 6 comments · Fixed by #3395
Assignees

Comments

@shane-tomlinson
Copy link

Copied from https://bugzilla.mozilla.org/show_bug.cgi?id=1221489


Reproducible with Firefox 43.0b1 build 2 (Build ID: 20151103023037) and 42.0RC build 2 (Build ID: 20151029151421)
Affected platforms: Mac OS X 10.10.4, Windows 7 64-bit and Ubuntu 14.04 32-bit

Steps to reproduce:

  1. Launch Firefox with a clean profile.

Expected result:
Back button disabled.

Actual result:
Back button is enabled on FirstRun page. Clicking on it reloads the page and 'Forward' button is visible for a second.

Additional notes:

  1. Not reproducible with latest 44.0a2 and 43.0a2 (from 2015-10-14), nor with latest Nightly 45.0a1.
  2. Also reproducible with 42 beta, but not with 39 beta - will investigate further for the regression range.
  3. As a side issue, when I navigate to any other URL from FirstRun page and Hit back button, the 'Go Forward' button vanishes. Should I file a separate issue or it's ok to be tracked here?
  4. This issue and the above one (Additional notes 3) are not reproducible on https://www.mozilla.org/en-US/firefox/42.0/firstrun/learnmore/
@shane-tomlinson shane-tomlinson added this to the FxA-0: quality milestone Nov 18, 2015
@shane-tomlinson
Copy link
Author

My thoughts from https://bugzilla.mozilla.org/show_bug.cgi?id=1221489#c5


(In reply to Alex Gibson [:agibson] from comment #4)

I'm not sure if there's anything we can do on the mozorg side to fix this
bug, it seems to be something in the iframe code that causes the history to
change?

Yes, the firstrun flow loads FxA to root - /. When / is opened, we look at whether the user is currently signed in. If the user does not have an FxA session, we redirect them to /signup, if they are, we send them to /settings. Both of these use history.pushState, taken care of by Backbone's router.

Perhaps a history.replaceState would be better for this situation.

@vladikoff
Copy link
Contributor

@shane-tomlinson
Copy link
Author

Re-opening in place of mozilla/fxa-bugzilla-mirror#94.

@shane-tomlinson
Copy link
Author

I'm wondering if this is a manifestation of https://bugzilla.mozilla.org/show_bug.cgi?id=301307. We are already using backbone's replace. In the iframe case, this calls location.replace instead of history.replaceState

@shane-tomlinson
Copy link
Author

This is a really annoying issue. What fixes the behavior in one version of Firefox breaks the back button in others.

The back button behavior is caused by two problems:

  1. When embedded in an iframe, we rely on hashchange events instead of using history.pushState.
  2. Firefox incorrectly adds a history item when using location.replace if the only portion of the URL to change is the hash.

The specific back button behavior outlined in this issue (firstrun flow) is fixed if we stop using hashchange events, and instead for pushState, but like everything in web development this is not without repercussions.

hashchange

Browser about:accounts firstrun iframed oauth
Firefox 43 (release) no back button until history.back() back button on startup back button on startup
Firefox 44 (beta) back button after first screen change, but broken back button on startup back button on startup
Firefox 45 (aurora) back button after first screen change, works correctly back button on startup back button on startup
Firefox 46 (nightly) back button after first screen change, works correctly back button on startup back button on startup

pushState

Browser about:accounts firstrun iframed oauth
Firefox 43 (release) no back button until history.back() No back button until history.back() No back button until history.back()
Firefox 44 (beta) back button after first screen change, broken back button after first screen change, works correctly back button after first screen change, works correctly
Firefox 45 (aurora) back button after first screen change, works correctly back button after first screen change, works correctly back button after first screen change, works correctly
Firefox 46 (nightly) back button after first screen change, works correctly back button after first screen change, works correctly back button after first screen change, works correctly

@shane-tomlinson
Copy link
Author

Changing from hashchange to pushState would cause a browser back button regression in Firefox <= 43, no back button is shown until after history.back(), which means users would be unable to use the browser's back button until they somehow navigate to the "forgot password" screen and hit the "back" link. Users can still navigate between screens using our secondary links so not all is lost, it's just the browser's back button that will be broken. Luckily for us, Firefox 44 is due out on Jan 26th. Our next train is tagged on January 25th and not released until Feb 1st. If we fix this behavior this train, we'll have a reasonable fix for 99% of users in a few weeks.

shane-tomlinson pushed a commit that referenced this issue Jan 18, 2016
The combination of using hashchange events and a browser bug caused
the browser back button to be enabled as soon as the user loaded
the firstrun flow. pushState does not suffer from this problem.
Use pushState instead of hashchange.

This fix has some repercussions on Fx <= 44. Read the table in the
referenced bug to find out more.

fixes #3296
shane-tomlinson pushed a commit that referenced this issue Jan 18, 2016
The combination of using hashchange events and a browser bug caused
the browser back button to be enabled as soon as the user loaded
the firstrun flow. pushState does not suffer from this problem.
Use pushState instead of hashchange.

This fix has some repercussions on Fx <= 43. Read the table in the
referenced bug to find out more.

fixes #3296
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 a pull request may close this issue.

2 participants