1538762 - Navigating on TreeHerder gets viewport stuck in strange position, unable to scroll to see whole page

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[Kartikaya Gupta (email:[e-mail])]

+++ This bug was initially created as a clone of Bug #1531057 +++

The patch on bug 1531057 fixed the STR from that bug, but I can still encounter situations where scrolling the page doesn't let me view everything I'm supposed to be able to.

For example:

1. go to https://treeherder.mozilla.org/#/jobs?repo=try&author=kgupta%40mozilla.com&fromchange=d48562c4bbe65f02f34ad0cb72b2e192daf727eb
2. zoom in towards the failed build jobs
3. click on one of the failed build jobs
4. scroll down to the bottom where the panel shows the details
5. scroll leftwards as far as you can

expected: you can scroll all the way to the left of the page
actual: you can't. when i just tried it, I can see the "ob:" part of the "Job:" text in the bottom-left panel, but the J and left margin is cut off. If I zoom out, at some point it lets me see that bit.

Also the bottom panel is clipped vertically (only ~3 lines of the failure summary are visible). In general the position of that bottom panel seems to fluctuate a lot the more I navigate around and click on stuff.

Comment 1

[Botond Ballo [:botond]]

Testing with today's nightly, I can't reproduce the "unable to scroll leftwards" issue.

Could you check what the minimap looks line when the problem occurs? Does it look like this, with the red rectangle (layout viewport) being shifted to the right of where it should be?

I can reproduce the issue of the bottom panel being very short vertically, but it doesn't appear to be a graphics / APZ issue - it looks like something is just sizing it to be that short. Its contents are scrollable in that short vertical scroll port.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Screenshot_20190326-070300.png

I hit a more severe case this morning, where I started Nightly and it session-restored the TH tab I had last time, scrolled over to the right, and I was unable to scroll leftward. I turned on the minimap and grabbed a screenshot, attached.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

... and I can't repro the STR from comment 0 anymore :/

Comment 4

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)

Created attachment 9053558 [details]
Screenshot_20190326-070300.png

I hit a more severe case this morning, where I started Nightly and it session-restored the TH tab I had last time, scrolled over to the right, and I was unable to scroll leftward. I turned on the minimap and grabbed a screenshot, attached.

This one looks like bug 1531057 comment 10.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

(In reply to Botond Ballo [:botond] from comment #4)

This one looks like bug 1531057 comment 10.

I'm pretty sure both of my {before,after} builds were more recent than March 19, so maybe there's more ways to trigger that one. I'll try and find STR if it happens again. In the meantime it might be worth landing some logging in-tree that I can grab via logcat if it happens again.

Comment 6

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #5)

I'm pretty sure both of my {before,after} builds were more recent than March 19, so maybe there's more ways to trigger that one. I'll try and find STR if it happens again. In the meantime it might be worth landing some logging in-tree that I can grab via logcat if it happens again.

Sorry, I meant it looks like the second paragraph from bug 1531057 comment 10:

It does not fix the bug in cases where the scroll restoration is triggered via the "session store", which includes closing the tab followed by "Undo close tab", or restarting the browser. I'm afraid the fix for those scenarios will have to come from bug 1516056.

That is, this will be fixed by bug 1516056, which hasn't landed yet, but I don't unfortunately have an idea for a 67-targeted fix for it :/

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Ah ok. Let's wait until bug 1516056 is in then, and see if there's anything left to do for this bug (i.e. if I still run into any issues).

Comment 8

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #6)

That is, this will be fixed by bug 1516056, which hasn't landed yet, but I don't unfortunately have an idea for a 67-targeted fix for it :/

Actually, depending on how badly we want it, with sufficient imagination I could probably cook up a 67-targeted fix.

Comment 9

[Botond Ballo [:botond]]

Attachment: Bug 1538762 - Ensure the Android session store respects the layout scroll range. r=kats

Comment 10

[Botond Ballo [:botond]]

It turned out to require very little imagination. Not sure why I haven't thought of this before.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/112ab0109e86
Ensure the Android session store respects the layout scroll range. r=kats

Comment 12

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/112ab0109e86

Comment 13

[Botond Ballo [:botond]]

Comment on attachment 9054054 [details]
Bug 1538762 - Ensure the Android session store respects the layout scroll range. r=kats

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1498812
• User impact if declined: After zooming in on certain pages and restarting the browser (or certain other workflows that trigger scroll restoration from session store), the user can get stuck in a state where they can't scroll back to the top / left of the page.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a small fix targeted to the session store code, that avoids most of the regression risk that the full fix (which is in bug 1516056, and targets 68) has.
• String changes made/needed:

Comment 14

[Pascal Chevrel:pascalc]

Comment on attachment 9054054 [details]
Bug 1538762 - Ensure the Android session store respects the layout scroll range. r=kats

Targeted fix for a 67 regression, covered by tests, uplift approved for our next 67 beta, thanks.

Comment 15

[Andreea Pavel [:apavel]]

Got a conflict when trying to uplift this

mozilla@ubuntu ~/mozilla-unified beta(0) $ hg graft -er 112ab0109e86
grafting 532330:112ab0109e86 "Bug 1538762 - Ensure the Android session store respects the layout scroll range. r=kats"
merging mobile/android/tests/browser/chrome/test_session_scroll_position.html
merging toolkit/components/sessionstore/SessionStoreUtils.cpp
warning: conflicts while merging mobile/android/tests/browser/chrome/test_session_scroll_position.html! (edit, then use 'hg resolve --mark')

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d2592c21a5c4

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Resolved the conflict by uplifting the test patches from bug 1531057 which got added after its behavior altering patch.

Comment 18

[Botond Ballo [:botond]]

Thank you, Sebastian!