1544325 - Mouse wheel scrolling in Nightly on Grafana dashboards is highly erratic

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Chris Siebenmann]

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.

Comment 1

[Chris Siebenmann]

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.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

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.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

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

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: A reduced test case

Comment 5

[Chris Siebenmann]

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.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

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

Comment 7

[Hiroyuki Ikezoe (:hiro)]

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

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: A mochitest doesn't fail on trunk

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.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1544325 - 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

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.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/325527265543