Closed
Bug 1251370
Opened 8 years ago
Closed 8 years ago
In Fennec with APZ, after a real touch event, mouse wheel events are ignored.
Categories
(Firefox for Android Graveyard :: Toolbar, defect)
Firefox for Android Graveyard
Toolbar
Tracking
(firefox48 fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: rbarker, Assigned: eflores)
References
Details
Attachments
(1 file)
2.23 KB,
patch
|
rbarker
:
review+
|
Details | Diff | Splinter Review |
To reproduce, tap the screen with a finger. Attempt to scroll when a mouse wheel. The result is the mouse wheel has no effect. Click any mouse button once and the mouse wheel will work again.
Reporter | ||
Updated•8 years ago
|
Summary: In Fennec with APZ, after a touch real touch event, mouse wheel events are ignored. → In Fennec with APZ, after a real touch event, mouse wheel events are ignored.
Reporter | ||
Comment 1•8 years ago
|
||
This bug is caused by Bug 1246984. What is going on is that downtime is per pointer, so a finger tap sets the mLastDownTime to the current time, but the mouse pointer device did not have a down event so it's event.getDownTime() is now before mLastDownTime so that any new scroll events from the mouse are ignored until the mouse pointer device has a new down event (from a mouse click) so that its event.getDownTime() is not the same as mLastDownTime.
Reporter | ||
Comment 2•8 years ago
|
||
That should read: "so that its event.getDownTime() is *NOW* the same as mLastDownTime."
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → edwin
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #8730733 -
Flags: review?(bugmail.mozilla)
Comment 4•8 years ago
|
||
Comment on attachment 8730733 [details] [diff] [review] 1251370.patch Review of attachment 8730733 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/java/org/mozilla/gecko/gfx/NativePanZoomController.java @@ +180,5 @@ > final int action = event.getActionMasked(); > + if (action == MotionEvent.ACTION_SCROLL) { > + if (event.getDownTime() >= mLastDownTime) { > + mLastDownTime = event.getDownTime(); > + } else if ((InputDevice.getDevice(event.getDeviceId()).getSources() & InputDevice.SOURCE_TOUCHPAD) == InputDevice.SOURCE_TOUCHPAD) { I think you can just do event.getSource() == InputDevice.SOURCE_TOUCHPAD. I actually don't know if using bitwise & on getSources() is correct; I thought that only made sense if you used SOURCE_CLASS_* rather than SOURCE_*. (i.e. if you checked for SOURCE_CLASS_POSITION it might make sense, but in this case you really only care about SOURCE_TOUCHPAD). That being said rbarker is probably a better person to review this patch at this point.
Attachment #8730733 -
Flags: review?(bugmail.mozilla) → review?(rbarker)
Reporter | ||
Comment 5•8 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4) > Comment on attachment 8730733 [details] [diff] [review] > 1251370.patch > > Review of attachment 8730733 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: > mobile/android/base/java/org/mozilla/gecko/gfx/NativePanZoomController.java > @@ +180,5 @@ > > final int action = event.getActionMasked(); > > + if (action == MotionEvent.ACTION_SCROLL) { > > + if (event.getDownTime() >= mLastDownTime) { > > + mLastDownTime = event.getDownTime(); > > + } else if ((InputDevice.getDevice(event.getDeviceId()).getSources() & InputDevice.SOURCE_TOUCHPAD) == InputDevice.SOURCE_TOUCHPAD) { > > I think you can just do event.getSource() == InputDevice.SOURCE_TOUCHPAD. > > I actually don't know if using bitwise & on getSources() is correct; I > thought that only made sense if you used SOURCE_CLASS_* rather than > SOURCE_*. (i.e. if you checked for SOURCE_CLASS_POSITION it might make > sense, but in this case you really only care about SOURCE_TOUCHPAD). > This patch fixes the issue for me. According to the docs, the bitwise & is correct, however I think the suggestion to use "event.getSource() == InputDevice.SOURCE_TOUCHPAD" instead is cleaner if that works (I tried it on my end with a blue tooth mouse and seemed to work).
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4) > This patch fixes the issue for me. According to the docs, the bitwise & is > correct, however I think the suggestion to use "event.getSource() == > InputDevice.SOURCE_TOUCHPAD" instead is cleaner if that works (I tried it on > my end with a blue tooth mouse and seemed to work). Tried this earlier, but sadly the bb keyboard reports SOURCE_MOUSE on its events. Eugh.
Reporter | ||
Comment 7•8 years ago
|
||
Comment on attachment 8730733 [details] [diff] [review] 1251370.patch Review of attachment 8730733 [details] [diff] [review]: ----------------------------------------------------------------- Please try :kats suggestion of "event.getSource() == InputDevice.SOURCE_TOUCHPAD" but otherwise looks good to me.
Attachment #8730733 -
Flags: review?(rbarker) → review+
Reporter | ||
Comment 8•8 years ago
|
||
(In reply to Edwin Flores [:eflores] [:edwin] from comment #6) > (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4) > > This patch fixes the issue for me. According to the docs, the bitwise & is > > correct, however I think the suggestion to use "event.getSource() == > > InputDevice.SOURCE_TOUCHPAD" instead is cleaner if that works (I tried it on > > my end with a blue tooth mouse and seemed to work). > > Tried this earlier, but sadly the bb keyboard reports SOURCE_MOUSE on its > events. Eugh. Sorry, didn't see this before posting review. Okay, then land it as is then.
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6da26abc6e24
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•