Consider capturing wheel events to the DOM element that the current wheel transaction started over

NEW
Unassigned

Status

()

3 years ago
3 years ago

People

(Reporter: mstange, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox41 affected)

Details

(Reporter)

Description

3 years ago
At the moment, as you scroll over a page with the mouse wheel, the mouse moves over different elements of the page, and the wheel events are sent to whatever element is under the mouse at the time.

I think it would be a good idea to attach the wheel event target to the current wheel transaction, and then send all wheel events of the current transaction to that same DOM element.

(It's not clear what we should do if the element is removed from the document during the transaction - maybe send the event to the first ancestor that still is in the document?)

Doing this would avoid a few problems with APZ. One problem we have now is the following: If you start scrolling over an element that doesn't preventDefault() any wheel events, and then during the wheel transaction the mouse suddenly comes across an element that does preventDefault(), then some wheel events will still result in APZ scrolling until APZ is notified by the content thread that it needs to stop. Locking wheel events to the element that the scroll started over would work around this problem in most cases.

I'm going into a little more detail about this in bug 1013412 comment 5.

I brought up this idea in bug 1166871 comment 4 but I'm not sure if anybody understood what I meant.

(In reply to Olli Pettay [:smaug] from comment #7)
> Parent process could indeed capture wheel events in case wheel events are
> targeted to content process.

I don't really understand this. What does "capture" mean in this case, and how would the event flow work as a result?

> We do have the transaction model for wheel events so that during the
> transaction same thing is being
> scrolled,

Right. We scroll the same thing, but the DOM events might go to different elements during the scroll.

> but I'm not sure how that works with apz.

In APZ we have the concept of an InputBlock which mirrors the behavior of ESM wheel transactions, using timeouts and mouse moves to abort the current transaction, see the code around https://dxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/InputBlockState.cpp#303

> In fact, why doesn't that work in parent process already?

Not sure what this is referring to.
This is e10s+apz bug, right? Since for non-apz there is the mousewheel transaction thing.
No, this is for the non-apz case. Consider this example: http://people.mozilla.org/~kgupta/bug/1168182.html. Put your mouse cursor over the scrollable div and use the wheel/trackpad to scroll. Note how the events are fired at whatever element happens to be under the cursor at the time. What markus is suggesting is that the events should always be targeted to the same element, just like touch events are.
That's what I was asking in bug 1166871 comment 5. I don't know the answer.
(Reporter)

Comment 5

3 years ago
Yeah, that is the big question. Can we try it out before we enable APZ and wait for regression reports?

I've been trying to come up with ideas for possible problems but wasn't very successful.
I think web content mostly uses wheel events for three purposes:
 (1) Implementing its own page scrolling
 (2) Implementing scrolling for specific scrollboxes
 (3) Zooming (e.g. maps)

In each of these cases, each wheel event that's used for custom scrolling / zooming should be preventDefault()ed. I have, however, seen cases of (2) and (3) where this was forgotten, maybe because it was only ever tested on pages where the page itself wasn't scrollable. As a result, you'd get double scrolling over the affected areas.

From now on I'm using "new behavior" to mean what would happen if we did what I suggest in this bug.

In case (1), the new behavior wouldn't be different from the old behavior because all wheel events on the page would be preventDefault()ed, so we'd never start a wheel transaction, and there wouldn't be any capturing.

In cases (2) and (3) if you start scrolling outside the affected area, the new behavior would be different because you'd just keep scrolling over the scrollbox and the scrollbox's wheel event handler wouldn't ever see any event.

In cases (2) and (3) with preventDefault() if you start scrolling inside the affected area, the new behavior won't be different because all wheel events are preventDefault()ed, so we don't start a wheel transaction, so we don't do any capturing.

In cases (2) and (3) *without* preventDefault() if you start scrolling inside the affected area, the new behavior will be very different because you'll keep zooming / scrolling the scrollbox even after the mouse has left the affected area due to page scrolling.
Masayuki, any thoughts on this?
Flags: needinfo?(masayuki)
Sorry for the delay to reply because I'm still not sure if this is right.

In strictly speaking, this is wrong. "wheel" event doesn't a scroll event. It notifies web apps of device operation by users. So, even if an element isn't scrollable, "wheel" events should also be fired on it.

If we lock wheel event target to a scrollable element, it must provide better UX for the users if any elements handle the event as a trigger to scroll a pseudo scrollable element even though it doesn't conform to UI Events (a.k.a. D3E) spec.

However, if wheel event handler may be used different purpose, the new behavior may break the web. For example, wheel event handlers may be implemented for moving character on games, counting up or down numbers, etc.

Anyway, the behavior shouldn't depend on e10s nor APZ. So, when you do this, I'd like you to change current behavior and enable it only  on non-release builds for waiting bug reports.
Flags: needinfo?(masayuki)
Note that we wouldn't actually move the target to a scrollable element (at least, that's not what I had in mind). If your mouse is positioned inside a scrollable element, the event target would still be the element under the cursor, and it would stay that same target for the duration of the transaction. The event target may be different from the containing scrollable element, which would be the thing that scrolls.

That being said we have another solution for bug 1166871 so I'm not convinced there's a need for this change now (or at least there's no driving force to get it done).
(Reporter)

Comment 9

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> That being said we have another solution for bug 1166871 so I'm not
> convinced there's a need for this change now (or at least there's no driving
> force to get it done).

The driving force from my side is more about bug 1013412:

(In reply to Markus Stange [:mstange] from comment #0)
> Doing this would avoid a few problems with APZ. One problem we have now is
> the following: If you start scrolling over an element that doesn't
> preventDefault() any wheel events, and then during the wheel transaction the
> mouse suddenly comes across an element that does preventDefault(), then some
> wheel events will still result in APZ scrolling until APZ is notified by the
> content thread that it needs to stop.

Implementing this bug would reduce the chances of double-processing the event.

Oh, and I just thought of a user experience reason to implement this:

Once we have overscrolling on OS X, I hope the user will feel much more in control of the scrolled page than they do now, because the finger movements will always translate into a page movement until the fingers are lifted from the touchpad. If the page now suddenly interrupts this scroll gesture, for example by calling preventDefault() during overscroll (and presumably triggering a snap back animation?), this feeling of control is thwarted.
(Reporter)

Comment 10

3 years ago
(In reply to Markus Stange [:mstange] from comment #9)
> Oh, and I just thought of a user experience reason to implement this:
> 
> Once we have overscrolling on OS X, I hope the user will feel much more in
> control of the scrolled page than they do now, because the finger movements
> will always translate into a page movement until the fingers are lifted from
> the touchpad. If the page now suddenly interrupts this scroll gesture, for
> example by calling preventDefault() during overscroll (and presumably
> triggering a snap back animation?), this feeling of control is thwarted.

Actually, never mind this part. During overscroll, the wheel event position from the perspective on the page won't change, because the layout scroll position will just be the edge of the page (without overscroll), so the event target won't change during overscroll anyway, and we don't need this bug for that.
(In reply to Markus Stange [:mstange] from comment #9)
> The driving force from my side is more about bug 1013412:
> 
> (In reply to Markus Stange [:mstange] from comment #0)
> > Doing this would avoid a few problems with APZ. One problem we have now is
> > the following: If you start scrolling over an element that doesn't
> > preventDefault() any wheel events, and then during the wheel transaction the
> > mouse suddenly comes across an element that does preventDefault(), then some
> > wheel events will still result in APZ scrolling until APZ is notified by the
> > content thread that it needs to stop.
> 
> Implementing this bug would reduce the chances of double-processing the
> event.

Ok, fair enough. For completeness - it would reduce the chances, but wouldn't drop them to zero because a listener might not do preventDefault() on some invocations and might do it on later invocations and then we'd have the same issue.
At least the default handler should capture wheel transactions. For example,
http://www.gizmodo.jp/2016/03/batmangoogle.html
Once the mouse cursor is over the embedded Google street view, suddenly the wheel scroll will stop and street view will zoom out. This is very frustrating for me.
Also, the current behavior is inconsistent. If the embedded street view is replaced with a scrollable textarea, iframe, or whatever, the surrounding element will capture wheel events.
You need to log in before you can comment on or make changes to this bug.