Open Bug 1327933 Opened 8 years ago Updated 2 years ago

[e10s][APZ] wheel event.preventDefault() doesn't work the first time when it fires, sometimes results in scrolling page instead of custom scrollable area

Categories

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

48 Branch
defect

Tracking

()

People

(Reporter: arni2033, Unassigned)

References

(Depends on 1 open bug)

Details

(Whiteboard: [gfx-noted][see comment 2, third paragraph, for user-observable behaviour])

>>> My Info: Win7_64, Nightly 49, 32bit, ID 20160526082509 STR_1: 1. Open url [1] 2. Hover mouse over the white area on the page, right above the gray area 3. Rotate mouse wheel down once 4. In less than 1s after scrolling has stopped, rotate mouse wheel down once 5. In less than 1s after scrolling has stopped, rotate mouse wheel down once AR: Step 4 - mouse is placed above gray area, but the page scrolls down. Step 5 - no visible action ER: Either X or Y X) Step 4 - no visible action. Step 5 - N/A, because in Step 4 scrolling doesn't start Y) Step 4 - the page should be scrolled down. Step 5 - the page should be scrolled down (see STR_2) STR_2: (related; reference of not good, but consistent behavior STR_1 -> ER -> Y) 1. Open url [2] 2. Hover mouse over the white area on the page, right above the gray area 3. Rotate mouse wheel down once 4. In less than 1s after scrolling has stopped, rotate mouse wheel down once 5. In less than 1s after scrolling has stopped, rotate mouse wheel down once AR: Steps 4,5 - mouse is placed above gray area, but the page scrolls down, just as expected Note: Firefox has a special mechanism that prevents scrolling of scrollable area for a few seconds after scrolling of its ancestor has stopped - just in case user wants to continue scrolling said ancestor. I can easily see how the same mechanism can be applied to preventing of wheel events: The only logical way to implement such a feature is to disallow event.preventDefault() for wheel events for a few seconds after scrolling of an ancestor of event.target has stopped. However, AR in STR_1 is nothing like this, and it doesn't fit into any logical expectations. > [1] data:text/html,<div onwheel="var e=arguments[0]; console.log(e); e.preventDefault()"><style>body{margin:500px 0px 0px}div{height:10000px;width:100%;background:gray} > [2] data:text/html,<div><span><style>body{margin:500px 0px 0px}div{height:10000px;width:100%;background:gray;overflow:scroll;}span{display:block;height:200000px;width:100%;background:lightgray}
No longer blocks: 1277113
Component: Untriaged → Panning and Zooming
Product: Firefox → Core
I agree that this behaviour is "broken", and we have had bug 1151654 on file for it. However, we have not encountered any real-world websites that are impacted by this behaviour. Do you have any examples of affected websites? It's also important to know what impacted websites are doing in order to figure out the best way to make the preventDefault behaviour consistent without breaking a lot of things.
Flags: needinfo?(arni2033)
First of all, I expect (X) in STR_1 (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1) > I agree that this behaviour is "broken", and we have had bug 1151654 on file for it. > However, we have not encountered any real-world websites that are impacted by this behaviour. Secondly, if by some (minor) chance you'll find my argument sufficient enough: fix this bug in this bug report, instead of stealing it to bug 1151654. Those people already had their chance. Or at least hide this comment, and pretend you've never seen it, and I'll wait until bug 1151654 is resolved > Do you have any examples of affected websites? Yes, I noticed this on most of websites with "custom scrollable area" on the page that overrides normal scrolling via e.preventDefault() and scrolls contents of that area. There're many examples of such scrollable areas - see further updates in bug 1257815 and 3 most important connected bugs. So, the bug is that if I rotate mouse wheel once and mouse is placed above a "custom scrollable area" on the page, then the next tick of mouse wheel scrolls the page instead of the scrollable area. > It's also important to know what impacted websites are doing in order to figure out the best way > to make the preventDefault behaviour consistent without breaking a lot of things. I don't see why you haven't made decision. "Just do what non-e10s does, and nothing will be broken", it's a golden rule that works in most cases (not all cases, but it applies here).
Flags: needinfo?(arni2033)
(In reply to arni2033 [Please stop 'improving' Firefox] from comment #2) > First of all, I expect (X) in STR_1 Agreed, that is the correct behaviour. > Yes, I noticed this on most of websites with "custom scrollable area" on the > page that overrides > normal scrolling via e.preventDefault() and scrolls contents of that area. > There're many examples of > such scrollable areas - see further updates in bug 1257815 and 3 most > important connected bugs. > So, the bug is that if I rotate mouse wheel once and mouse is placed above a > "custom scrollable area" > on the page, then the next tick of mouse wheel scrolls the page instead of > the scrollable area. Thanks, this is helpful. > > It's also important to know what impacted websites are doing in order to figure out the best way > > to make the preventDefault behaviour consistent without breaking a lot of things. > I don't see why you haven't made decision. "Just do what non-e10s does, and > nothing will be broken", it's a golden rule that works in most cases (not > all cases, but it applies here). The problem is that "what non-e10s does" is to not have APZ at all. We can't both have async scrolling and respect preventDefault() on every event, because knowing if an event is preventDefault()'d by definition involves running script on the main thread, and is not async. If we want wheel scrolling to be async (I know *you* don't, but in general we do, and we have gotten a lot of feedback in favor of it) then we need to hack around this preventDefault() business in some way. The compromise we have so far is to always respect it on the first wheel event in a transaction, and then if a later event has preventDefault() called on it we try and abort the async scroll as soon as possible. However as you discovered it's not instant and sometimes we scroll a few extra events. > (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1) > enough: fix this bug in this bug report, instead of stealing it to bug > 1151654. Those people already had their chance. Or at least hide this "those people" includes me. It's not like we don't want to fix the bug. It's just that there's a cost to fixing it, and that cost is losing APZ. I realize that from your point of view this bug should be fixed and you don't care about having APZ, but we have to look at the bigger picture and see what is best solution for all our users. Not all users have the same experience as you, and not all users have the same opinion as you.
Depends on: 1151654
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All
Summary: [e10s][APZ] wheel event.preventDefault() doesn't work the first time when it fires → [e10s][APZ] wheel event.preventDefault() doesn't work the first time when it fires, sometimes results in scrolling page instead of custom scrollable area
Whiteboard: [gfx-noted][see comment 2, third paragraph, for user-observable behaviour]
Version: Trunk → 48 Branch
Do pointer events solve this problem?
I don't think so; pointer events don't deal with wheel events - just "pointer" types like mouse and touch.
Interesting. I wonder if there is room for something like a 'wheel-action: none' property for situations like this.
See also: https://bugzilla.mozilla.org/show_bug.cgi?id=1304044 Implemented auxclick - but event.preventDefault() does not work properly when used with e10s ON An example: https://wicg.github.io/auxclick/
(In reply to janekptacijarabaci from comment #7) > See also: > https://bugzilla.mozilla.org/show_bug.cgi?id=1304044 > > Implemented auxclick - but event.preventDefault() does not work properly > when used with e10s ON > An example: > https://wicg.github.io/auxclick/ I don't think this has anything to do with this bug. Please file a new bug for the issue you are seeing. Thanks.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.