558915 - Scroll wheel zooming breaks after pan

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Description

[Matt Brubeck (:mbrubeck)]

Steps to reproduce (requires a mouse with a scroll wheel):
1. Open any web page.
2. Drag to pan the web page.
3. Use the scroll wheel to zoom the page.

Expected: Page zooms.
Actual:   Nothing happens.

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: patch 1: bad "this" in timeout handler

"this" in the timeout callback is set to the global object, so writes to a global variable "pendingEvent" instead of the intended instance variable.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch 2: cleanup ungrab

ungrab is never called with its first argument null, so this is dead code.  I see no no need calling it with null in the future either.  (If the InputHandler was not grabbed, then there should be no ignored events to restore.)

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch 3: don't make nested ungrab calls on mousemove

http://hg.mozilla.org/mobile-browser/rev/6629e9516bb1 adds support for nested grab/ungrab calls, but requires one call to ungrab() for every call to grab().

MouseModule disobeys this since it calls grab many times (on mousedown *and* on each mousemove), but calls ungrab only once (on mouseup).  This always leaves the InputHandler in a "grabbed" state and prevents calls to any other handler.

This patch prevents nested grabs in MouseModule.

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch 3 (alternate): remove nested grab code

6629e9516bb1 greatly complicated grab/ungrab by adding support for nested calls and event replay.  Nine months later, we are still not using either of those features, so most of this code has never been used or tested.  Maybe we should just remove it?

On the other hand, bug 558320 might actually use these features.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 438594 [details] [diff] [review]
patch 3 (alternate): remove nested grab code

Lets try to remove this cruft

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/24feb65ec1f3
http://hg.mozilla.org/mobile-browser/rev/eee1ca00a56a

Comment 7

[Fabrice Desré]

Ungrabbing with replay events can actually be useful. I'm using it in my experimental gesture support. The "too much grabbing" issue needs to be fixed in MouseModule (that probably only should grab when panning) but the nested feature was actually needed.

Comment 8

[Aakash Desai [:aakashd]]

verified FIXED on build:

Mozilla/5.0 (Macintosh; U; Intel Mac OSX 10.5; en-US; rv:1.9.2.4) Gecko/20100413 Namoroka/3.6.4pre Fennec/1.1a2pre