490873 - Dragging in chrome causes an extra click event to be sent at end of drag

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ben Combee (:bcombee)]

Noticed by gavin, needs to be fixed for beta 2.

Comment 1

[Ben Combee (:bcombee)]

Attachment: Remove extra click on end of a drag inside chrome

This works by changing our ignoreNextClick one-time flag into an allowNextClick one-time flag.  We set it both when we generate a click or when we get a mouseUp not related to a pan.  I've tested with bookmarks, awesomebar, and URL bar.

Comment 2

[Ben Combee (:bcombee)]

This patch applies on top of Brad's patch in bug 441585

Comment 3

[Ben Combee (:bcombee)]

Comment on attachment 375211 [details] [diff] [review]
Remove extra click on end of a drag inside chrome

Gavin found a problem -- this prevents clicking in content!  I'm working on it.

Comment 4

[Ben Combee (:bcombee)]

Attachment: Try #2 - cleaned up some data structures I noticed, moved click filtering to InputHandler

A little more code change than I originally thought, but I noticed issues with data members being defined in the prototypes, so I cleaned that up while moving things around.  This requires the new version of the drag patch; I updated that about 15 minutes ago.

Comment 5

[Ben Combee (:bcombee)]

Attachment: Updated to apply on latest version of 441585

Comment 6

[Ben Combee (:bcombee)]

Attachment: Update again for changes in 441585

Comment 7

[Ben Combee (:bcombee)]

Attachment: Update to apply to trunk

Comment 8

[Ben Combee (:bcombee)]

The patch does two things:

1) Change the click handling so the InputHandler only allows a click through when it's been requested.  This solves the problem with extra clicks in chrome.

2) fixes some leftover issues with data items being declared in the prototypes for classes

Comment 9

[Ben Combee (:bcombee)]

The reason this patch updates the top-level InputHandler is because of a cascading event problem.

If we only do click prevention in ChromeInputHandler, then we have the potential of capturing a click that's intended for content.  However, I didn't want to change ChromeInputHandler to filter out events that are destined for content as that means there's more coupling between this class and ContentDragHandler.

Therefore, to solve the problem, I push clicking up into InputHandler so it becomes a privileged event that's always filtered out unless the click is intended to go through.  Content handler generates clicks, so it can allow them.  Chrome handler also generates clicks, so it can allow them too.

It's not really ideal.  I think to fix this better we'd need to add a whole filter mechanism to InputHandler to control the scope of what handlers saw what events.  This starts to sound like we're reinventing the whole event bubbling mechanism though.

Comment 10

[Ben Combee (:bcombee)]

Attachment: Update for tree rot

Comment 11

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

Comment on attachment 380866 [details] [diff] [review]
Update for tree rot

carrying stuart's r+

Comment 12

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

http://hg.mozilla.org/mobile-browser/rev/3b157de16a8a

Comment 13

[Aakash Desai [:aakashd]]

verified FIXED checking R<->L scroll on the search bar on builds:

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20090821
Fennec/1.0b3pre

and

Mozilla/5.0 (Macintosh; U; Intel Mac OSX 10.5; en-US; rv:1.9.2a2pre)
Gecko/20090808 Fennec/1.0b3pre