|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
59 bytes, text/x-review-board-request
|Details | Review|
59 bytes, text/x-review-board-request
|Details | Review|
Nightly 52 on a nexus 4. Go to https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FIREFOX_50_0b8_RELEASE&tochange=FIREFOX_50_0b9_RELEASE, zoom in and click on one of the csets. Go back. ER: zoom and scroll position restored. AR: page is zoomwd out
I bisected this using mozregression. While bisecting I found another issue where sometimes after hitting back, the zoom value would be restored but the scroll position was not restored properly. I counted those builds as "good" since this bug is specifically about zoom. The bisection got as far as this before barfing: INFO: Last good revision: 8a6045d14d6bd348a3b5bfeb55a9321e680cc93e (2015-08-24) INFO: First bad revision: 04b8c412d9f58fb6194c58dcaa66bf278bbd53cf (2015-08-25) INFO: Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8a6045d14d6bd348a3b5bfeb55a9321e680cc93e&tochange=04b8c412d9f58fb6194c58dcaa66bf278bbd53cf So it's most likely either bug 1197592 or bug 1180267.
status-firefox49: --- → wontfix
status-firefox50: --- → wontfix
status-firefox51: --- → fix-optional
status-firefox52: --- → affected
OS: Unspecified → Android
Priority: -- → P3
Hardware: Unspecified → All
Version: 50 Branch → 43 Branch
status-firefox53: --- → affected
status-firefox51: fix-optional → wontfix
status-firefox52: affected → fix-optional
a year ago
status-firefox54: --- → affected
status-firefox55: --- → affected
kats, still on your radar? I notice no one is assigned. We can let this fall off the platform triage if it's considered backlog.
status-firefox52: fix-optional → wontfix
status-firefox53: affected → wontfix
status-firefox54: affected → fix-optional
Yeah let's consider it backlog.
status-firefox55: affected → fix-optional
So the actual issue is quite simple: The resolution is restored on the PresShell before the first paint, so it subsequently gets clobbered by the MobileViewportManager's default zoom calculation.
Assignee: nobody → jh+bugzilla
Comment on attachment 8882002 [details] Bug 1312605 - Part 0 - Fix indentation in MVM. https://reviewboard.mozilla.org/r/153058/#review158326 Thanks for fixing this! (And for putting in a separate patch!)
Attachment #8882002 - Flags: review?(bugmail) → review+
Comment on attachment 8882003 [details] Bug 1312605 - Part 1 - Don't clobber resolution changes that happen before first paint on Fennec. https://reviewboard.mozilla.org/r/153060/#review158328
Attachment #8882003 - Flags: review?(bugmail) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/8a07aa497968 Part 0 - Fix indentation in MVM. r=kats https://hg.mozilla.org/integration/autoland/rev/e1283673d436 Part 1 - Don't clobber resolution changes that happen before first paint on Fennec. r=kats
Backed out for failing mobile/android/tests/browser/chrome/test_session_scroll_position.html: https://hg.mozilla.org/integration/autoland/rev/c8ef01f4ce1070c574b69734031813c682195843 https://hg.mozilla.org/integration/autoland/rev/1f2c1539b344aee13dfddf8931b979e9429ad6af Failure log (Android was busted during the push for this bug): https://treeherder.mozilla.org/logviewer.html#?job_id=110619566&repo=autoland [task 2017-06-29T02:08:46.099383Z] 02:08:46 INFO - 218 INFO TEST-PASS | mobile/android/tests/browser/chrome/test_session_scroll_position.html | zoom restored correctly [task 2017-06-29T02:08:46.099766Z] 02:08:46 INFO - 219 INFO TEST-PASS | mobile/android/tests/browser/chrome/test_session_scroll_position.html | scrollX restored correctly [task 2017-06-29T02:08:46.100214Z] 02:08:46 INFO - 220 INFO TEST-PASS | mobile/android/tests/browser/chrome/test_session_scroll_position.html | scrollY restored correctly [task 2017-06-29T02:08:46.100630Z] 02:08:46 INFO - 221 INFO TEST-PASS | mobile/android/tests/browser/chrome/test_session_scroll_position.html | can go back [task 2017-06-29T02:08:46.101061Z] 02:08:46 INFO - 222 INFO Now waiting for pageshow event from browser [task 2017-06-29T02:08:46.101441Z] 02:08:46 INFO - 223 INFO Now waiting for scroll event from browser [task 2017-06-29T02:08:46.101746Z] 02:08:46 INFO - Buffered messages logged at 02:08:38 [task 2017-06-29T02:08:46.102051Z] 02:08:46 INFO - 224 INFO Received event pageshow from browser [task 2017-06-29T02:08:46.102460Z] 02:08:46 INFO - 225 INFO Received event scroll from browser [task 2017-06-29T02:08:46.103008Z] 02:08:46 INFO - Buffered messages finished [task 2017-06-29T02:08:46.103242Z] 02:08:46 INFO - 226 INFO TEST-UNEXPECTED-FAIL | mobile/android/tests/browser/chrome/test_session_scroll_position.html | undefined assertion name - got true, expected "zoom restored correctly" [task 2017-06-29T02:08:46.103615Z] 02:08:46 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:312:5 [task 2017-06-29T02:08:46.103885Z] 02:08:46 INFO - test_sessionStoreScrollPositionAndZoomLevel@chrome://mochitests/content/chrome/mobile/android/tests/browser/chrome/test_session_scroll_position.html:214:5 [task 2017-06-29T02:08:46.104530Z] 02:08:46 INFO - onFulfilled@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:69:15
I think I got "is" and "ok" confused there, but it's now even failing on try even with the correct test. Strangely enough earlier on I managed to get an unexpected pass with the test unchanged. Unfortunately I can't run tests locally right now, so will look into it next week when I get back.
(In reply to Jan Henning [:JanH] from comment #12) > I think I got "is" and "ok" confused there, but it's now even failing on try > even with the correct test ...because I did a try push in artifact mode (duh). Should be fine now...
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/cb4445c53902 Part 0 - Fix indentation in MVM. r=kats https://hg.mozilla.org/integration/autoland/rev/53ba9f0f61da Part 1 - Don't clobber resolution changes that happen before first paint on Fennec. r=kats
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Comment on attachment 8882003 [details] Bug 1312605 - Part 1 - Don't clobber resolution changes that happen before first paint on Fennec. Approval Request Comment [Feature/Bug causing the regression]: Bug 1180267 [User impact if declined]: Zoom level is not restored when navigating back/forward through a tab's session history [Is this code covered by automated tests?]: Partially. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: This just feeds resolution changes that happen before first paint into the same code path that is already being used by the session store. [String changes made/needed]: none
Attachment #8882003 - Flags: approval-mozilla-beta?
Comment on attachment 8882003 [details] Bug 1312605 - Part 1 - Don't clobber resolution changes that happen before first paint on Fennec. Nice, let's uplift the fix! I'm sure many people who use zoom (for example for accessibility/vision issues) will appreciate zoom level being sticky as they go back through the history.
Attachment #8882003 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-firefox55: fix-optional → fixed
Verified as fixed on build 56.0a1 (7-20) and 55.0b10 with devices: HTC 10 (Android 7.0) Honor 8 (Android 6.0). Following the steps from description and another pages, zoom was restored when going back in history.
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
status-firefox56: fixed → verified
You need to log in before you can comment on or make changes to this bug.