Closed Bug 1538762 Opened 6 months ago Closed 6 months ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox66 --- wontfix
firefox67 --- fixed
firefox68 --- fixed

People

(Reporter: kats, Assigned: botond)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

+++ 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.

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.

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.

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

(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.

(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.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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 :/

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).

Depends on: 1516056

(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.

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

Assignee: nobody → botond
Blocks: 1498812
No longer depends on: 1516056
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/112ab0109e86
Ensure the Android session store respects the layout scroll range. r=kats
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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:
Attachment #9054054 - Flags: approval-mozilla-beta?
No longer blocks: 1498812
Has Regression Range: --- → yes
Has STR: --- → yes
Regressed by: 1498812

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.

Attachment #9054054 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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')

Flags: needinfo?(botond)

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

Flags: needinfo?(botond)

Thank you, Sebastian!

You need to log in before you can comment on or make changes to this bug.