Closed Bug 1657627 Opened 4 years ago Closed 4 years ago

Mouse and scroll position disagree on charcod.es

Categories

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

defect

Tracking

()

VERIFIED FIXED
82 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox79 --- disabled
firefox80 --- verified disabled
firefox81 --- verified
firefox82 --- verified

People

(Reporter: mozbz, Assigned: kats)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

Attached video Video of 'Actual Results' (deleted) —

STR

  1. Create a fresh profile
  2. Visit https://charcod.es/#letter_like
  3. Scroll down and click a symbol in the last line. Information about the symbol will appear.
  4. If necessary, scroll to the very bottom again.
  5. Click another symbol in the last few lines.

Expected Results:
When clicking a symbol in the last line, the scroll position should jump to bring the information panel to the bottom of the screen. Selecting other symbols should work as expected.

Actual Results:
The scroll position is not visually reset, but the page behaves as if it were - the mouse 'hover' activates items far above the cursor's actual position. This effect persists even with the DevTools Inspector as shown in the accompanying video.

Regression Range:
mozregression thinks this is regressed by bug1644271 (good: 61fccfec51e98f51d9865bf1ff5808016a948a71, bad:01a1753056cfcc36972d43791d0f6fe43a502480), which seems possible -- in Release 79 and DevEd 80 the problem is not present when the preference apz.mvm.force-enabled is set to 'false'. In Nightly 81 the issue always occurs whatever the preference. Tested on Win10 and Linux.

Thanks for filing! I can investigate.

Assignee: nobody → kats
Status: UNCONFIRMED → NEW
Ever confirmed: true
Regressed by: 1644271
Has Regression Range: --- → yes

Hm, looks like the main thread and compositor disagree about the scroll position. In the good version (prior to bug 1644271) clicking on letters also adjusts the scroll position so that the info box is at the bottom of the window. But after bug 1644271 it seems like the main thread scroll position gets updated while the compositor scroll position does not, resulting in the visual mismatch. And then doing some scrolling resyncs the compositor scroll position back to the main thread and things are good again.

Attached file Reduced test case (deleted) —

The content height shrinks and expands within a single vsync interval, but also forces the main-thread scroll position to be clamped to the reduced content height. After that is where things go out of sync. Attaching reduced testcase.

Severity: -- → S2
Priority: -- → P3

We seem to be hitting this codepath which prevents the main thread from clamping the APZ visual scroll position, which is why they end up out of sync. The comment above that block refers to bug 1543485 implementing a proper fix, and Botond is working on that. The fix for that bug may fix this issue too, or at least might unlock the fix for this. I'll think about this a bit more and see if there's a way to fix it in a localized way.

Depends on: 1543485

Discussed this with Botond and we can up with a simple fix for uplifting to beta; we can replace the GetIsViewportOverridden check for an apz.allow_zooming check. That will retain the originally intended behaviour of restricting the codepath to mobile. It won't fix it on desktop nightly though, since we have that pref turned on there - but Botond intends to fix bug 1543485 for the 81 nightly cycle and after that we should be able to fix this by removing the codepath entirely.

Here's a try push with that change and apz.allow_zooming turned to false, to ensure that this change won't break tests when uplifted to beta.
https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=5cfd73c8c6a5667750792a0e236db1fa1e9e359b

This restricts the workaround to mobile a bit better, since the
GetIsViewportOverridden() check returns true on desktop as of bug 1644271.

I intend to land this patch on 81 nightly and uplift to 80 beta. That way if we don't fix bug 1543485 in the nightly 81 cycle at least this patch will ride the trains to 81 beta, and we won't need additional work to prevent the bug from manifesting in 81 release.

Marking the bug leave-open since the patch won't fix it in nightly yet.

Keywords: leave-open

Comment on attachment 9168882 [details]
Bug 1657627 - Refine condition used in temporary workaround code. r?botond

Beta/Release Uplift Approval Request

  • User impact if declined: On some pages, it's possible to get into a state where effects based on mouse position are visually offset (see STR/video on this bug and bug 1650977). This state persists until the user scrolls the page.

Note this patch doesn't fix the issue on Nightly because Nightly has the apz.allow_zooming pref set to true. But the patch will fix the issue on Beta while we work on the more involved fix for Nightly.

  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: STR in comment 0, or use the reduced test case attached to the bug. Again - note that this patch landing on Nightly will have no effect, please only test on the beta build that contains the patch after uplift.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Simple condition swap from one that used to be true on Android only but no longer is, to one that is still true on Android only (on beta/release).
  • String changes made/needed: none
Attachment #9168882 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/528cf50c8b8f Refine condition used in temporary workaround code. r=botond

Comment on attachment 9168882 [details]
Bug 1657627 - Refine condition used in temporary workaround code. r?botond

approved for 80.0b7

Attachment #9168882 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Reproduced the issue using a Nightly build 81.0a1 (2020-08-11) on Windows 10.
Verified - Fixed in latest Beta 80.0b7 (build id: 20200810131809) using the steps from the description and verified also with the reduced test case attached on Windows 10 and Ubuntu 18.04.

I'm going to use this bug as the canonical version since there's a bunch of bugs filed now that are effectively the same root cause.

Verified that this is fixed in the latest nightly thanks to bug 1543485. I'll use this bug to land a test.

Apparently fixed by bug 1543485 in 81.

The uplift fixed from this bug (comment 13) fixed it in 80, not sure why it was marked "verified disabled". And bug 1543485 fixed it in 82 as well, so if we're marking 81 as fixed, we should mark 82 as fixed too.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #23)

The uplift fixed from this bug (comment 13) fixed it in 80, not sure why it was marked "verified disabled". And bug 1543485 fixed it in 82 as well, so if we're marking 81 as fixed, we should mark 82 as fixed too.

In Firefox 80, the code path with the issue is disabled when the pref apz.allow_zooming is false, which it is for Release but not Nightly. I think it makes sense to have a different status, because the patch that was uplifted wouldn't fix the issue on desktop Nightly. Instead, it disables it for Release.

(In reply to Mathew Hodson from comment #25)

In Firefox 80, the code path with the issue is disabled when the pref apz.allow_zooming is false, which it is for Release but not Nightly. I think it makes sense to have a different status, because the patch that was uplifted wouldn't fix the issue on desktop Nightly. Instead, it disables it for Release.

Ah, I see. That's a reasonable argument since the semantics of these flags aren't clearly defined anyway.

Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

Verified as fixed on Firefox 81.0b6 and on Firefox Nightly 82.0a1 (2020-09-03) on Windows 10 x64, Ubuntu 20.04 x64 and on MacOS 10.14.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+

I just had this issue regress on Nightly 82.0a1 (2020-09-13). It was the list of rooms on the Mozilla slack. See also bug 1664838.

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

Attachment

General

Creator:
Created:
Updated:
Size: