Closed Bug 524543 Opened 13 years ago Closed 13 years ago

Fast double taps sometimes cause clicks


(Firefox for Android Graveyard :: General, defect)

Not set


(Not tracked)



(Reporter: stechz, Assigned: stechz)



(1 file)

This is possibly a regression from fixing accidental content clicks.  If one of the clicks has not been parsed as a click (<X msecs have occurred), there will only be a single click.
Can we get an actual steps to reproduce here?
Double tap a link on the N900 really fast.  Sometimes it will do nothing, other times it will open the link.
Yep, I'm seeing this too on the n900 on today's nightly build:

Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091026

I dont think this is a regression as I couldn't find any bugs on file for this issue.
Flags: in-litmus?
This patch includes some refactoring, as any time we change how axis locking works or what constitutes a pan or a click, a lot of code is being touched outside of DragData.  This patch localizes the responsible code into DragData.  This patch feeds information into DragData (much like our kinetic controller) and then ask it questions like: is it a pan? what are the current panning coordinates?

Here's a quick guide to the changes:
> _onMouseUp: function _onMouseUp(evInfo) {
> -    let [sX, sY] = dragData.lockAxis(evInfo.event.screenX, evInfo.event.screenY);
> -
> -    if (dragData.dragging)
> -      this._doDragStop(sX, sY, !dragData.checkPan(sX, sY));
> +    if (dragData.dragging) {
> +      dragData.setDragPosition(evInfo.event.screenX, evInfo.event.screenY);
> +      let [sX, sY] = dragData.panPosition();
> +      this._doDragStop(sX, sY, !dragData.isPan());
> +    }

Lock axis is no longer used.  Instead, when mouse movement data is received, setDragPosition is called with the screen coordinates.  Then, dragData.isPan() and dragData.isClick() tells the event handler how to handle this drag.  The pan and click checks no longer take coordinates as arguments because setDragPosition now has that logic and sets appropriate object state.

>   _doDragMove: function _doDragMove(sX, sY) {
>     let dragData = this._dragData;
> -    let dX = dragData.sX - sX;
> -    let dY = dragData.sY - sY;
> +    let dX = dragData.prevPanX - sX;
> +    let dY = dragData.prevPanY - sY;

Previous pan coordinates are stored by dragData so that a delta can be calculated for the dragger.

setDragPosition now contains the needed logic for when the drag locks, how it locks, and what constitutes a pan.  The functional difference from before is that double taps no longer have to have clicks that take over 50 milliseconds.

madhava and I have extensively tested this patch by panning, double tapping, clicking on chrome things, and looking for accidental content clicks.  If this patch is too scary, I can hack together something where the functional changes from before are more obvious.
Assignee: nobody → webapps
Attachment #408887 - Flags: review?(21)
Comment on attachment 408887 [details] [diff] [review]
Changes to dragData

I like the refactoring part, and I'm not sure why but panning seems a bit faster.

This also seems to correct a long remaining bug that I'm seeing sometimes with quick quadruple click.

Very nice work!
Attachment #408887 - Flags: review?(21) → review+
Good refactor! Good result!

Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → B5
verified FIXED on tinderbox build:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2b2pre) Gecko/20091029 Namoroka/3.6b2pre Fennec/1.0b5
This would be better suited for a browser chrome test.
Flags: in-testsuite?
Flags: in-litmus?
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.