DOMMouseScroll doesn't fire on Mac trackpad with slow movements

RESOLVED WONTFIX

Status

()

Core
DOM: Events
RESOLVED WONTFIX
5 years ago
5 years ago

People

(Reporter: iangilman, Assigned: spohl)

Tracking

23 Branch
mozilla26
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox23 wontfix, firefox24+ verified, firefox25+ verified, firefox26 verified, firefox27 wontfix, firefox-esr17 unaffected)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
This was working fine with Firefox 22 and before; it broke with Firefox 23. To reproduce:

1. Go to http://lab.cubiq.org/iscroll/examples/simple/ with a Mac that has a trackpad (tested on Mac Pro Mountain Lion with Magic Trackpad).
2. Do the two-finger scroll gesture on the trackpad slowly. Note that nothing happens.
3. Do it more quickly; note that it does move (though not much).

You can see the error in http://comic-viewer-dev.appspot.com/view/brad/ (my site) as well.

If you use a normal mouse scroll wheel on Mac Firefox 23, or the Mac trackpad on another browser, the sites work fine.

Also normal scrolling pages work fine with the Mac trackpad on Firefox 23... clearly the scroll message is getting through; it's just that the DOMMouseScroll event isn't firing properly.

Here's an interesting case: http://openseadragon.github.io/ The zooming image on the page is using DOMMouseScroll, but the page itself is also tall enough that there's room for vertical scrolling. In this case the DOMMouseScroll seems to be getting through properly.

My theory is that there's some "optimization" where the DOMMouseScroll event doesn't fire when the page knows there's not enough room for vertical scrolling. This is bad news for sites like http://comic-viewer-dev.appspot.com/view/brad/ that use the scroll gesture for zooming.

Please let me know if you need additional information!
(Reporter)

Comment 1

5 years ago
I've created these bare-bones test pages to show the bug in action:

http://iangilman.com/comics/scroll-broken.html
http://iangilman.com/comics/scroll-kind-of-working.html

You can see clearly that DOMMouseScroll is tied to the page's available space for scrolling, which it shouldn't be.
Stephen, can you check whether this was caused by bug 673875?
Flags: needinfo?(spohl.mozilla.bugs)
(Assignee)

Comment 3

5 years ago
Bug 673875 isn't currently turned on due to bug 860493, which is waiting for bug 817700 to be fixed... Or did you mean a different bug number?
Flags: needinfo?(spohl.mozilla.bugs)
I actually do mean bug 673875, because I have the feeling that the widget changes from it still have an impact even when the feature is disabled.
(Assignee)

Comment 5

5 years ago
Hmm, that'd be bad. Let me check.
(Assignee)

Comment 6

5 years ago
Indeed, looks like bug 673875 is the culprit. I'll check if this is an easy fix. Otherwise, I'll revert the patches in bug 673875 for now, since the bounce behavior isn't turned on yet.
Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Blocks: 673875
(Reporter)

Comment 7

5 years ago
Glad to know the culprit has been found! Will we be able to get the fix in version 24 (or sooner) or does it have to ride through the whole release cycle? Sorry for the naive question... I'm out of touch with the process these days.

Also, in the meantime, any thoughts on how a webapp might work around this issue?

Thank you :-)
(Assignee)

Comment 8

5 years ago
Bug 673875 has just been backed out on inbound. After a few days, I'll request approval for uplift. Since this is a backout, I'm hopeful that we can uplift it to Fx24.
https://hg.mozilla.org/mozilla-central/rev/ce98deb07663
https://hg.mozilla.org/mozilla-central/rev/7c78f04b8640
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
(Reporter)

Comment 10

5 years ago
Awesome, thanks for the quick response! Having this in Fx24 would be great. I see it's currently marked for Fx26... I assume that's because you haven't gotten uplift approval yet.
(Assignee)

Comment 11

5 years ago
(In reply to Ian Gilman [:iangilman] from comment #10)
> I assume that's because you haven't gotten uplift approval yet.

Your assumption is correct. :-)
status-firefox24: --- → affected
status-firefox25: --- → affected
status-firefox26: --- → fixed
status-firefox-esr17: --- → unaffected
tracking-firefox24: --- → ?
tracking-firefox25: --- → ?

Updated

5 years ago
status-firefox23: --- → unaffected
tracking-firefox24: ? → +
tracking-firefox25: ? → +
(Assignee)

Comment 12

5 years ago
Created attachment 795017 [details] [diff] [review]
Part 1: Revert fd6c7792b048

This patch was r+'d in bug 673875. Moving it here to get approvals for aurora and beta and for better tracking.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 673875
User impact if declined: Scrolling can stop prematurely and feel broken on OSX.
Testing completed (on m-c, etc.): Has been on m-c for ~3 days. Local testing shows that the issue is fixed.
Risk to taking this patch (and alternatives if risky): low (backout)
String or IDL/UUID changes made by this patch: none
Attachment #795017 - Flags: review+
Attachment #795017 - Flags: approval-mozilla-beta?
Attachment #795017 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 13

5 years ago
Created attachment 795018 [details] [diff] [review]
Part 2: Revert 366e6a39d71a

This patch was r+'d in bug 673875. Moving it here to get approvals for aurora and beta and for better tracking.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 673875
User impact if declined: Scrolling can stop prematurely and feel broken on OSX.
Testing completed (on m-c, etc.): Has been on m-c for ~3 days. Local testing shows that the issue is fixed.
Risk to taking this patch (and alternatives if risky): low (backout)
String or IDL/UUID changes made by this patch: none
Attachment #795018 - Flags: review+
Attachment #795018 - Flags: approval-mozilla-beta?
Attachment #795018 - Flags: approval-mozilla-aurora?

Updated

5 years ago
Attachment #795017 - Flags: approval-mozilla-beta?
Attachment #795017 - Flags: approval-mozilla-beta+
Attachment #795017 - Flags: approval-mozilla-aurora?
Attachment #795017 - Flags: approval-mozilla-aurora+

Updated

5 years ago
Attachment #795018 - Flags: approval-mozilla-beta?
Attachment #795018 - Flags: approval-mozilla-beta+
Attachment #795018 - Flags: approval-mozilla-aurora?
Attachment #795018 - Flags: approval-mozilla-aurora+
(In reply to Ian Gilman [:iangilman] from comment #10)
> Awesome, thanks for the quick response! Having this in Fx24 would be great.
> I see it's currently marked for Fx26... I assume that's because you haven't
> gotten uplift approval yet.

Can you please help confirm the issue you reported is fixed now ? Thanks for you report and help on this issue :)

Updated

5 years ago
Keywords: checkin-needed
(Reporter)

Comment 16

5 years ago
(In reply to bhavana bajaj [:bajaj] from comment #14)
> Can you please help confirm the issue you reported is fixed now ? Thanks for
> you report and help on this issue :)

Just tested in nightly... yes, fixed! Thank you :-)
Isn't Firefox 23 actually affected by this bug, too?
(Assignee)

Comment 18

5 years ago
(In reply to Markus Stange [:mstange] from comment #17)
> Isn't Firefox 23 actually affected by this bug, too?

Right, Firefox 23 is affected.
status-firefox23: unaffected → affected
(Assignee)

Comment 19

5 years ago
Spoke with :bajaj and this is wontfix for Fx23, unfortunately. The fix will be released with Fx24.
status-firefox23: affected → wontfix
Duplicate of this bug: 911253
Keywords: verifyme
We don`t have a Mac with trackpad here so can you please help us verify that this issue is fixed on latest Aurora and latest Nightly?
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly//latest-mozilla-aurora/
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly//latest-mozilla-central/
Flags: needinfo?(ian)
(Reporter)

Comment 22

5 years ago
I have just now verified that it is fixed on the latest Nightly and Aurora, and that it is fixed in Firefox 24.
Flags: needinfo?(ian)
(In reply to Ian Gilman [:iangilman] from comment #22)
> I have just now verified that it is fixed on the latest Nightly and Aurora,
> and that it is fixed in Firefox 24.
Thanks a lot Ian.
If you have some sphere time and want to verify this on Firefox 25 beta 6 as well, just to properly close this bug it would be much appreciated :)
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly//25.0b6-candidates/build1/mac/en-US/
Status: RESOLVED → VERIFIED
status-firefox24: fixed → verified
status-firefox26: fixed → verified
Keywords: verifyme
(Reporter)

Comment 24

5 years ago
I have now verified that it is fixed in the Firefox beta you linked to.
Thanks again Ian.
status-firefox25: fixed → verified
(Assignee)

Comment 26

5 years ago
The latest patches for bug 673875 have reintroduced this bug, but it turns out that we may end up WONTFIX-ing it. Bringing the discussion over here from bug 926274, where I started the conversation with Masayuki.

From bug 926274:
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from bug 926274 comment #8)
> (In reply to Stephen Pohl [:spohl] from bug 926274 comment #7)
> > I have one concern however: We will inevitably break what's been reported in
> > bug 907275 again. On the test page [1], when scrolling the list,
> > ComputeScrollTarget returns NULL, which we take to mean that the ViewPort
> > has been overscrolled. This means that we stop the vertical scroll and cause
> > the page to bounce instead. I've confirmed that Safari does the same, with
> > the difference that while the page is bouncing, the item list is scrolling
> > too. We can't do both (bounce and scroll) at the same time, since we're
> > currently using snapshots for the bounce effect until we've implemented the
> > AsyncPanZoomController for OSX (which will take some time). Masayuki, can
> > you tell me if this is an acceptable breakage?
> 
> I don't understand well your question. The testcase looks like not working
> fine with current Nightly build. Additionally, on my environment, swipe
> animation isn't performed. Do I need to change something in about:config?
> # The iscroll.js should handle wheel event rather than DOMMouseScroll event,
> though.

Right, the latest patches for bug 673875 have reintroduced this problem, so the testcase is broken on current Nightly. Swipe animations (and bounce behavior) can be turned on by setting browser.snapshots.limit to a value above 0 in about config (the default will be 5, see bug 860493).

Basically, the question right now is if we expect this test case to work in the future, keeping in mind that it also misbehaves in Safari, or whether the content should be changed to handle wheel events instead (would this fix it?). The difference in behavior between Safari and Firefox is explained in my earlier comment above (lack of AsyncPanZoomController on OSX).
Status: VERIFIED → REOPENED
status-firefox27: --- → affected
Flags: needinfo?(masayuki)
Resolution: FIXED → ---
(Assignee)

Comment 27

5 years ago
On closer inspection it seems like iscroll.js handles both DOMMouseScroll and mousewheel events. Removing DOMMouseScroll did not have any effect that I could see.
Maybe it's just missing an event.preventDefault() call?
(Assignee)

Comment 29

5 years ago
(In reply to Markus Stange [:mstange] from comment #28)
> Maybe it's just missing an event.preventDefault() call?

I thought I've checked that, but I had to realize that the e.preventDefault() in the onBeforeScrollStart in iscroll.js is seemingly not being called when it should. Calling e.preventDefault() at the top of the _wheel function works well however. Thanks Markus!

Based on the fact that Safari exhibits a similar problem and that this can be fixed with a call to e.preventDefault() in the _wheel function I'm going to close this as WONTFIX for now. Please feel free to reopen if I'm missing anything here.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
status-firefox27: affected → wontfix
Flags: needinfo?(masayuki)
Resolution: --- → WONTFIX
(Reporter)

Comment 30

5 years ago
Should this bug really be WONTFIX? The original bug has indeed been fixed... the only thing that's WONTFIX is the little bit of discussion at the bottom here. 

Also, regarding that discussion, let me know if there's anything I can do to verify that things are still working properly. I have a couple of projects which use the scrollwheel in this way, such as http://driftory.com.
(Assignee)

Comment 31

5 years ago
(In reply to Ian Gilman [:iangilman] from comment #30)
> Should this bug really be WONTFIX? The original bug has indeed been fixed...
> the only thing that's WONTFIX is the little bit of discussion at the bottom
> here.

Ever since bug 673875 has landed again, the original issue has resurfaced. The thing is that it hasn't propagated to aurora, beta and release yet. If you check nightly [1] you will see the original issue again.

> Also, regarding that discussion, let me know if there's anything I can do to
> verify that things are still working properly. I have a couple of projects
> which use the scrollwheel in this way, such as http://driftory.com.

This appears to be broken right now (at least intermittently). You will want to follow the advice in comment 28 and comment 29  and call event.preventDefault(). This will make sure that the page doesn't bounce when scrolling.

[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/firefox-27.0a1.en-US.mac.dmg
(In reply to Stephen Pohl [:spohl] from comment #31)
> (In reply to Ian Gilman [:iangilman] from comment #30)
> > Also, regarding that discussion, let me know if there's anything I can do to
> > verify that things are still working properly. I have a couple of projects
> > which use the scrollwheel in this way, such as http://driftory.com.
> 
> This appears to be broken right now (at least intermittently). You will want
> to follow the advice in comment 28 and comment 29  and call
> event.preventDefault(). This will make sure that the page doesn't bounce
> when scrolling.

I guess that it's not enough. The script should listen D3E "wheel" event rather than legacy "DOMMouseScroll" event. Firefox 17 ESR has already implemented D3E wheel event. So, it's available since over a year. Note that Google Chrome will support it soon, IE 8 and later have already supported it.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #32)
> (In reply to Stephen Pohl [:spohl] from comment #31)
> > (In reply to Ian Gilman [:iangilman] from comment #30)
> > > Also, regarding that discussion, let me know if there's anything I can do to
> > > verify that things are still working properly. I have a couple of projects
> > > which use the scrollwheel in this way, such as http://driftory.com.
> > 
> > This appears to be broken right now (at least intermittently). You will want
> > to follow the advice in comment 28 and comment 29  and call
> > event.preventDefault(). This will make sure that the page doesn't bounce
> > when scrolling.
> 
> I guess that it's not enough. The script should listen D3E "wheel" event
> rather than legacy "DOMMouseScroll" event. Firefox 17 ESR has already
> implemented D3E wheel event. So, it's available since over a year. Note that
> Google Chrome will support it soon, IE 8 and later have already supported it.

Sorry, IE 9 and later. IE 8 only supports "mousewheel".
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #32)
> (In reply to Stephen Pohl [:spohl] from comment #31)
> > (In reply to Ian Gilman [:iangilman] from comment #30)
> > > Also, regarding that discussion, let me know if there's anything I can do to
> > > verify that things are still working properly. I have a couple of projects
> > > which use the scrollwheel in this way, such as http://driftory.com.
> > 
> > This appears to be broken right now (at least intermittently). You will want
> > to follow the advice in comment 28 and comment 29  and call
> > event.preventDefault(). This will make sure that the page doesn't bounce
> > when scrolling.
> 
> I guess that it's not enough. The script should listen D3E "wheel" event
> rather than legacy "DOMMouseScroll" event.

Why is it not enough? I remember that, when I implemented MozMousePixelScroll, we tried to ensure that calling preventDefault only on the DOMMouseScroll event would still be enough to stop all scrolling default handling. Is that no longer the case?
(In reply to Markus Stange [:mstange] from comment #34)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #32)
> > (In reply to Stephen Pohl [:spohl] from comment #31)
> > > (In reply to Ian Gilman [:iangilman] from comment #30)
> > > > Also, regarding that discussion, let me know if there's anything I can do to
> > > > verify that things are still working properly. I have a couple of projects
> > > > which use the scrollwheel in this way, such as http://driftory.com.
> > > 
> > > This appears to be broken right now (at least intermittently). You will want
> > > to follow the advice in comment 28 and comment 29  and call
> > > event.preventDefault(). This will make sure that the page doesn't bounce
> > > when scrolling.
> > 
> > I guess that it's not enough. The script should listen D3E "wheel" event
> > rather than legacy "DOMMouseScroll" event.
> 
> Why is it not enough? I remember that, when I implemented
> MozMousePixelScroll, we tried to ensure that calling preventDefault only on
> the DOMMouseScroll event would still be enough to stop all scrolling default
> handling. Is that no longer the case?

DOMMouseScroll is fired when accumulated scroll amount becomes over than 1 line. I.e., only wheel event and MozMousePixelScroll may be fired if user tries to scroll very slowly. It causes bug 681200.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #35)
> DOMMouseScroll is fired when accumulated scroll amount becomes over than 1
> line. I.e., only wheel event and MozMousePixelScroll may be fired if user
> tries to scroll very slowly. It causes bug 681200.

Can we instead fire it before the first pixel scroll event? I think this bug also causes slow scrolling on Google Docs (e.g. https://docs.google.com/spreadsheet/ccc?key=0Asj8iLTl0K0UdDcxemJEWVVXWTlKek1wR09iS2FsOXc&pli=1#gid=2 ) to fail... Or is this a different bug?
(In reply to Markus Stange [:mstange] from comment #36)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #35)
> > DOMMouseScroll is fired when accumulated scroll amount becomes over than 1
> > line. I.e., only wheel event and MozMousePixelScroll may be fired if user
> > tries to scroll very slowly. It causes bug 681200.
> 
> Can we instead fire it before the first pixel scroll event? I think this bug
> also causes slow scrolling on Google Docs (e.g.
> https://docs.google.com/spreadsheet/
> ccc?key=0Asj8iLTl0K0UdDcxemJEWVVXWTlKek1wR09iS2FsOXc&pli=1#gid=2 ) to
> fail... Or is this a different bug?

I've not tested it yet, I guess that when DOMMouseScroll event is fired is not matter. Even if we dispatch it before first MozMousePixelScroll, following events will be only wheel and MozMousePixelScroll. All web apps which handle mouse wheel should handle new wheel event instead of legacy events.
I agree, but that's easy for us to say... it still means we break existing apps, which we should try to avoid.
(In reply to Markus Stange [:mstange] from comment #38)
> I agree, but that's easy for us to say... it still means we break existing
> apps, which we should try to avoid.

We've not changed the behavior except we supported high resolution scroll on Mac. I have no idea to avoid breaking compatibility on some web apps which were broken by the change.
(Reporter)

Comment 40

5 years ago
Why does DOMMouseScroll have to break? Isn't it possible to keep it working and phase it out gracefully? 

At any rate, I've updated http://driftory.com to use "wheel" and preventDefault, so it's working now in both Nightly and Release. That all seems to work. 

There's a good deal of legacy code out there that uses DOMMouseScroll, though; it would be great if we could avoid breaking them if possible.
(In reply to Ian Gilman [:iangilman] from comment #40)
> Why does DOMMouseScroll have to break? Isn't it possible to keep it working
> and phase it out gracefully? v

DOMMouseScroll event cannot represent small amount scroll less than 1 line. It was enough for old (slow) machines because high resolution scroll was too expensive since it needs higher frequency repaint. In these days, the cost of high frequency repaint doesn't matter because of faster machines. Therefore, Mac OS X 10.5 (IIRC) and Windows Vista supported high resolution wheel event for smoother scroll. We implemented it with MozMousePixelScroll which indicates scroll amount in pixels. At this time, DOMMouseScroll was stayed for compatibility. However, its preventDefault() *cannot* prevent default actions of MozMousePixelScroll between 2 DOMMouseScroll events. This is the reason why old web application had to be broken by the change.

> it would be great if we could avoid breaking them if possible.

I have no idea to avoid the issues because it's too difficult to decide which MozMousePixelScroll event is killed by a call of DOMMouseScroll event.

Anyway, using new wheel event improves web applications. It indicates the scroll amount in lines, pixels or pages (see deltaMode attribute) clearly (Legacy mousewheel event doesn't indicate the good scroll amount). Additionally, all web browsers will support wheel event soon. So, web application's code will be simpler than now.
(Reporter)

Comment 42

5 years ago
Okay, thank you for the explanation. I certainly am glad to be moving to a more standard scrollwheel event!
You need to log in before you can comment on or make changes to this bug.