Closed Bug 1544325 Opened 2 years ago Closed 2 years ago

Mouse wheel scrolling in Nightly on Grafana dashboards is highly erratic

Categories

(Core :: Layout, defect, P1)

Desktop
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: cks+mozilla, Assigned: hiro)

References

(Regression)

Details

Attachments

(3 files)

Starting in recent Nightly versions, mouse wheel scrolling on Grafana dashboards (at least) has become quite erratic instead of smooth and continuous. At best it stutters; at worst the dashboard will refuse to scroll (it will scroll down briefly then jump right back up). A public Grafana dashboard that exhibits the problem is accessible at https://play.grafana.org/. I can't see any relevant errors in the browser console when trying play.grafana.org (there are some rejected tracking requests).

This happens both in the currently available Nightly and in a version built right now from the development tree. It didn't happen in a version built on April 5th.

Running mozgression, I ended up with a report of:
12:12.31 INFO: Narrowed inbound regression window from [b500d17a, fb721c0c] (3 builds) to [b500d17a, 2b8493e3] (2 builds) (~1 steps left)
12:12.31 INFO: No more inbound revisions, bisection finished.
12:12.31 INFO: Last good revision: b500d17a3bf5523d70fe27d246a59ec7074cf35b
12:12.31 INFO: First bad revision: 2b8493e37c626b53fe8a3c0852573943412af809
12:12.31 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b500d17a3bf5523d70fe27d246a59ec7074cf35b&tochange=2b8493e37c626b53fe8a3c0852573943412af809

This points to bug #1531228. There is already a reported regression bug here, bug #1544006. After examining the changeset, I toggled layout.css.scroll-snap-v1.enabled to False in about:config and the problem went away.

Component: DOM: Events → Layout
Flags: needinfo?(hikezoe)
Regressed by: 1531228

More precisely, this was regressed by bug 1373835, especially https://hg.mozilla.org/mozilla-central/rev/ab5811341092 .

I need to debug to know what's going on there.

Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
Flags: needinfo?(hikezoe)
Priority: -- → P1
Regressed by: 1373835
No longer regressed by: 1531228

On my environment, the issue happans only if I change layout.css.devPixelsPerPx value. So it seems rounding error happens somewhere.

Attached file A reduced test case

The system I experienced this on and tested Nightly on is a Linux system with a 4K 'HiDPI' display. However I was using completely clean and unmodified profiles, both in my initial test of Nightly and in my mozregression run (as far as I know from the latter). I do have some HiDPI-related things in my X environment and environment variables that Nightly might have been picking up on. If you want me to check for or report specific things, let me know what.

The site calls Element.scrollTop with the same value of Element.scrollTop in a callback of scroll event for some reason. In such cases, with particular layout.css.devPixelsPerPx values (I did use 1.75) the current scroll top position (current) and the given scroll top position (pt) in ScrollFrameHelper::ScrollToCSSPixels will differ unfortunately. So I think we should revive currentCSSPixels checks against aScrollPosition in the function

(In reply to Chris Siebenmann from comment #5)

The system I experienced this on and tested Nightly on is a Linux system with a 4K 'HiDPI' display. However I was using completely clean and unmodified profiles, both in my initial test of Nightly and in my mozregression run (as far as I know from the latter). I do have some HiDPI-related things in my X environment and environment variables that Nightly might have been picking up on. If you want me to check for or report specific things, let me know what.

Yeah, HiDPI definitely causes similar effects just like the layout.css.devPixelsPerPx value.

Anyways, thanks for reporting this issue, I think this issue is less noticeable on normal sites, but it's a big regression. :)

I tried to write a mochitest to emulate the situation where the issue happens on https://play.grafana.org/. But unfortunately, the test doesn't fail as expected.

Presumably, mouse wheel events should be processed in a special condition, but I have no idea about it. (I used synthesizeWheel in a rAF and callbacks in setTimeout). So I will go without any tests.

FWIW, here is the test case I wrote.

There are conditions that the given scroll position in app units differs from
the current position in app units even if those values are the same in CSS
pixels. Setting Element.scrollTop with Element.scrollTop in a callback of
scroll event is one of such cases.

The change here looks messy, but I am totally unsure that we can clamp the
values before initializing the range for the DISABLE_SNAP case, so I'd like to
preserve the behavior there. Once we ship the new scroll snap, we can drop
the DISABLE_SNAP case there so that it could be less messy at the time.

Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/325527265543
Clamp the app units' scroll position to the current position if the given scroll position is the same as the current scroll position in CSS pixels. r=botond
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.