Zoom not restored when going back in history

VERIFIED FIXED in Firefox 55



Firefox for Android
2 years ago
10 months ago


(Reporter: kats, Assigned: JanH)



43 Branch
Firefox 56
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox49 wontfix, firefox50 wontfix, firefox51 wontfix, firefox52 wontfix, firefox53 wontfix, firefox54 wontfix, firefox55 verified, firefox56 verified)


MozReview Requests


Submitter Diff Changes Open Issues Last Updated
Error loading review requests:


(2 attachments)

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:

So it's most likely either bug 1197592 or bug 1180267.
status-firefox49: --- → wontfix
status-firefox50: --- → wontfix
status-firefox51: --- → fix-optional
status-firefox52: --- → affected
Keywords: regressionwindow-wanted
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
See Also: → bug 1265818


a year ago
Blocks: 1265818
See Also: bug 1265818
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

Comment 4

11 months ago
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 hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 7

11 months ago
Comment on attachment 8882002 [details]
Bug 1312605 - Part 0 - Fix indentation in MVM.


Thanks for fixing this! (And for putting in a separate patch!)
Attachment #8882002 - Flags: review?(bugmail) → review+

Comment 8

11 months ago
Comment on attachment 8882003 [details]
Bug 1312605 - Part 1 - Don't clobber resolution changes that happen before first paint on Fennec.

Attachment #8882003 - Flags: review?(bugmail) → review+
Comment hidden (mozreview-request)

Comment 10

11 months ago
Pushed by mozilla@buttercookie.de:
Part 0 - Fix indentation in MVM. r=kats
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:


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
Flags: needinfo?(jh+bugzilla)

Comment 12

11 months ago
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.
Flags: needinfo?(jh+bugzilla)

Comment 13

11 months ago
(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...
Comment hidden (mozreview-request)

Comment 15

11 months ago
Pushed by mozilla@buttercookie.de:
Part 0 - Fix indentation in MVM. r=kats
Part 1 - Don't clobber resolution changes that happen before first paint on Fennec. r=kats
Last Resolved: 11 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Comment 17

11 months ago
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-firefox54: fix-optional → wontfix

Comment 19

11 months ago
status-firefox55: fix-optional → fixed
Flags: in-testsuite+
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-firefox55: fixed → verified
status-firefox56: fixed → verified
You need to log in before you can comment on or make changes to this bug.