Closed
Bug 1337940
Opened 7 years ago
Closed 7 years ago
URL not properly restored on Reddit after navigating to the next page
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox54 verified)
VERIFIED
FIXED
Firefox 54
Tracking | Status | |
---|---|---|
firefox54 | --- | verified |
People
(Reporter: JanH, Assigned: JanH)
References
Details
Attachments
(3 files)
This is about the session store part of this bug report. +++ This bug was initially created as a clone of Bug #1337264 +++ User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0 Build ID: 20160606113944 Steps to reproduce: 1. Navigate to https://m.reddit.com/ 2. Scroll to bottom; tap "NEXT" button at bottom right to navigate to the next page of content 3. Tap location bar to notice the URL has updated to reflect the new page (eg, https://www.reddit.com/?count=25&page=1&after=t3_5sf6ns) 4. Kill Firefox from Android task manager 5. Restart Firefox Actual results: * After navigating to the second page, the location bar display is not updated properly, showing only https://www.reddit.com/ until I tap it, when it displays the actual URL of the current page * After Firefox is restarted, the front page that I navigated away from is restored Expected results: * The location should have updated to display the correct URL without having to tap to select the location * The page I was on when Firefox was killed should have been restored I suspect the problem is related to a javascript page transition on the Reddit site, but I have not investigated this, so I could be wrong.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8836410 [details] Bug 1337940 - Part 1 - Capture session store tab data on history listener notifications. https://reviewboard.mozilla.org/r/111834/#review113414
Attachment #8836410 -
Flags: review?(ahunt) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8836864 [details] Bug 1337940 - Part 2 - Make session store form data test work again. https://reviewboard.mozilla.org/r/112162/#review115650
Attachment #8836864 -
Flags: review?(ahunt) → review+
Pushed by mozilla@buttercookie.de: https://hg.mozilla.org/integration/autoland/rev/0172497c1024 Part 1 - Capture session store tab data on history listener notifications. r=ahunt https://hg.mozilla.org/integration/autoland/rev/dd8c7da15822 Part 2 - Make session store form data test work again. r=ahunt
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0172497c1024 https://hg.mozilla.org/mozilla-central/rev/dd8c7da15822
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Comment 8•7 years ago
|
||
backed this out for tc-M(31) perma failure, like https://treeherder.mozilla.org /logviewer.html#?job_id=79217617&repo=autoland&lineNumber=1937 https://hg.mozilla.org/mozilla-central/rev/c53368e7d1a3 https://hg.mozilla.org/mozilla-central/rev/9a9db410f207 https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=&fromchange=a437b95a0de44c025192f5d2a98c13bc0feb6a90&filter-searchStr=Android%204.3%20API15%2B%20debug%20Mochitests%20executed%20by%20TaskCluster%20test-android-4.3-arm7-api-15%2Fdebug-mochitest-31%20tc-M(31)&tochange=69dc3f79527d7cef2d3036756ca43bbf1a2f31ff Sorry we are not sure which patch causes this failure, so I backed your patches out. Please help to check it, thanks.
Status: RESOLVED → REOPENED
Flags: needinfo?(jh+bugzilla)
Resolution: FIXED → ---
Assignee | ||
Comment 9•7 years ago
|
||
That test seems to be causing a lot of location and presumably also history changes. Previously, the session store simply ignored those, but now it tracks them as well, so presumably the added overhead of capturing the tab state each time is enough to make this test time out on a debug build. I'll try and see if maybe folding bug 1339206 into this or anything else could reduce the impact here or whether we have to bite the bullet and increase the timeout.
Flags: needinfo?(jh+bugzilla)
Assignee | ||
Comment 10•7 years ago
|
||
So once again it turn out that the emulator is slooow and a debug build even sloooower. test_parse_rule consists of loads of sub-tests (more than 100 I think) and each new sub-test means a navigation and a new session history entry. We already start the test with something like 17 history entries and quickly run into the limit set by browser.sessionhistory.max_entries at 50 entries. At that point, collecting the history takes almost 1 s per call, so you can imagine the overhead this adds. Since these navigations don't change the page title, the session store previously didn't collect any history at all here. Bug 1341810 might help, so we wouldn't have to collect the full tab history all the time, although probably not all that much - once we run into the max_entries limit, we're also starting to get OnHistoryPurge notifications, where we can't avoid a full collect because we don't know precisely what has been purged. Besides, it'll probably be a while before that bug gets implemented. So in the meantime I'll try and see whether I can get lowering browser.sessionhistory.max_entries for this test only to have any immediate effect and if not it's probably going to be requestLongerTimeout().
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=714826874ab90473864d96114bc5330a2ac5005d
Updated•7 years ago
|
Attachment #8840764 -
Flags: review?(matt.woodrow) → review?(dbaron)
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8840764 [details] Bug 1337940 - Part 3 - Restrict the amount of session history we keep around during test_parse_rule. https://reviewboard.mozilla.org/r/115042/#review117012 ::: layout/style/test/test_parse_rule.html:256 (Diff revision 1) > +SpecialPowers.setIntPref("browser.sessionhistory.max_entries", 4); > + > +SimpleTest.registerCleanupFunction(function() { > + SpecialPowers.clearUserPref("browser.sessionhistory.max_entries"); > +}); The concept here seems fine to me, but I believe it's considered preferable to use SpecialPowers.pushPrefEnv rather than using setIntPrefand clearUserPref directly (particularly since prefs take a little time to take effect in e10s, if I recall correctly). So unless there's a reason you think this advice is wrong, could you restructure this to use pushPrefEnv instead? See: http://searchfox.org/mozilla-central/rev/90d1cbb4fd3dc249cdc11fe5c3e0394d22d9c680/testing/specialpowers/content/specialpowersAPI.js#998
Attachment #8840764 -
Flags: review?(dbaron) → review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8840764 [details] Bug 1337940 - Part 3 - Restrict the amount of session history we keep around during test_parse_rule. https://reviewboard.mozilla.org/r/115042/#review117198 r=dbaron. Thanks.
Attachment #8840764 -
Flags: review?(dbaron) → review+
Comment 20•7 years ago
|
||
Pushed by mozilla@buttercookie.de: https://hg.mozilla.org/integration/autoland/rev/4285ead0e57a Part 1 - Capture session store tab data on history listener notifications. r=ahunt https://hg.mozilla.org/integration/autoland/rev/6fa30faf3791 Part 2 - Make session store form data test work again. r=ahunt https://hg.mozilla.org/integration/autoland/rev/3e542f18ed62 Part 3 - Restrict the amount of session history we keep around during test_parse_rule. r=dbaron
Comment 21•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4285ead0e57a https://hg.mozilla.org/mozilla-central/rev/6fa30faf3791 https://hg.mozilla.org/mozilla-central/rev/3e542f18ed62
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Comment 22•7 years ago
|
||
Verified as fixed with the latest Nightly 54.0a1 build (2017-03-02). Device: HTC 10 (Android 6.0.1).
Status: RESOLVED → VERIFIED
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•