Closed Bug 1066157 Opened 10 years ago Closed 10 years ago

Synthetic click after touchend event dispatched on target even if moved beyond original coordinates

Categories

(Firefox for Android Graveyard :: Toolbar, defect)

All
Android
defect
Not set
normal

Tracking

(fennec+)

RESOLVED DUPLICATE of bug 788073
Tracking Status
fennec + ---

People

(Reporter: miketaylr, Assigned: bnicholson, Mentored)

References

()

Details

I'm terrible at bug titles, please edit so it makes more sense.

This issue was originally reported at webcompat.com: http://webcompat.com/issues/171 as a compat issue with time.com.

I've got it reduced down to https://miketaylr.com/bzla/wc-171-2.html.

STR:

1) Go to https://miketaylr.com/bzla/wc-171-2.html
2) In the "Tap 1" box, touch on the right side (to the right of the dotted line), or touch anywhere in "Tap 2" box.

Expected: toggle method that adds a class that increases the "right" property happens exactly one and the box moves to the left.
Actual: no matter where you tap, toggle is called twice and it quickly moves over then back (see "log").

For comparison, Chrome Mobile, iOS 7 Safari and Firefox OS (whatever latest Flame v2 image I have) all do the following:

Touching on the right side of Tap 1 results in a single toggle. Touching on the left side results in two toggles. Tapping anywhere in Tap 2 results in a single toggle. The crucial difference between Tap 1 and Tap 2 is the Tap 1 box is shifted half of it's width to the left while Tap 2 is shifted more than 100% of it's width. 

In Firefox for Android it doesn't matter where the element ends up--the synthetic click is dispatched on the div and it toggles again.
tracking-fennec: --- → ?
Summary: Synthetic click after touchend event dispatched on target even if it's moved → Synthetic click after touchend event dispatched on target even if moved beyond original coordinates
I can repro. Certainly an odd bug. I checked on B2G to see what happens, and the code in TabChild definitely sends the mousemove/down/up combo to trigger the click event, but somewhere in the bowels of gecko a decision is made to not actually send the click event. For some reason this decision isn't hit on fennec, so the click event does end up getting dispatched.

Also, based on my conceptual understanding of how things should work I would expect Fennec's behaviour to be correct and everybody else (including B2G) to be wrong.
OS: Mac OS X → Android
Hardware: x86 → All
EventStateManager dispatches click if mousedown and mouseup happen on the same element.

Mike, do you get click event elsewhere, not in .rail-handle elements? Since I think click should happen
on the parent of .rail-handle elements, and that is the expected behavior here I think.

Does Fennec miss a layout flush after touchend?
Oh, I think I know what's going on. Normally the click event will get dispatched, but it will get dispatched to the body because the element has already shifted out from under it. In Fennec we do this business with retargeting the mouse events [1] to deal with fuzzy clicks and that dispatches the mouse events to the new location of the element even though it's moved far away.

We really should get rid of that code and try to use the PositionedEventTargeting stuff from gecko instead.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=7854e1b5d181#4919
> Mike, do you get click event elsewhere, not in .rail-handle elements?

I hadn't thought to test that, I was just clicking coordinate properties on the event object itself. If I tweak the test case to log event.target and have body listen for clicks Chrome, iOS and FxOS are indeed dispatching the event to the body while Fennec does it to the moved .rail-handle.
Wes/Kats - Can you mentor Brian on this?
Assignee: nobody → bnicholson
tracking-fennec: ? → +
Flags: needinfo?(wjohnston)
Flags: needinfo?(bugmail.mozilla)
OS: Android → Mac OS X
Hardware: All → x86
Sure. In a nutshell what we want to do is kill off the code in browser.js that fuzzily tries to find a touch target, and ensure that the relevant code in gecko is used instead. If this results in regressions, then the code in Gecko should be updated to deal with those cases better.

One chunk of code in browser.js that does the touch fuzzing is at [1]. Ideally we should be able to get rid of that if statement and the call to elementFromPoint, and just always use the aEvent.target because that will Just Work (TM).

In the discussion on bug 1021804 wes mentioned that there was another chunk of code in the contextmenu handler, I believe at [2], that serves a similar function. This should probably also be removed, although I don't know that code as well.

The code in Gecko that performs the touch fuzzing is in layout/base/PositionedEventTargeting.cpp. I *think* that code should already be being invoked for touch events in Fennec, via the call site at [3].

Hopefully that gives you enough to start poking around the code and familiarizing yourself with it; let me know if you have questions.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=7854e1b5d181#4804
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=7854e1b5d181#2353
[3] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?rev=5db436c928aa#7098
Mentor: bugmail.mozilla
Flags: needinfo?(bugmail.mozilla)
OS: Mac OS X → Android
Hardware: x86 → All
We tried to do this once in bug 788073 but never got it landed. We also will still need to remove the code that uses the highlightedElement for clicks at:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#4915

Then hope that the mouse events we send the DOMWindowUtils are fuzzed by the platform. Do you know if that's the case kats? Other wise if you tap near a button (that doesn't move) we'll highlight it, but then fire the touch in some blank space nearby.
Flags: needinfo?(wjohnston) → needinfo?(bugmail.mozilla)
Yeah, I believe if you set the right flags (inputsource = MOZ_SOURCE_TOUCH) on the DOMWindowUtils call it will fuzz them.
Flags: needinfo?(bugmail.mozilla)
Might as well try to get that bug landed then, as it will automatically fix this.
Depends on: 788073
Bug 788073 fixes this, so marking as a duplicate.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.