Closed Bug 973348 Opened 6 years ago Closed 6 years ago

Element highlighted on a tap should always receive the click event

Categories

(Firefox for Android :: General, defect)

28 Branch
Other
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
firefox27 --- wontfix
firefox28 --- affected
firefox29 --- affected
firefox30 --- affected
fennec 30+ ---

People

(Reporter: kats, Assigned: wesj)

References

Details

Attachments

(2 files)

This has been a longstanding issue with Fennec. When the user taps, we pick an element and set it active on the touchstart. Once the tap gesture is complete we possibly pick another element and send it the click. I often run into this problem on my bugmail dashboard where i click a link, and the :active styles get apploed, making it look like it activated. However, nothing happens.

We should fix this by always dispatching the click to the same element we make active.
Added some logging to the touch redirection and pushed some try builds:

https://tbpl.mozilla.org/?tree=Try&rev=597476e79f3f

I do notice that we use multiple client rects to determine IF we need to move the point, but then just use rect[0] to move them. That should work fine, but doesn't really move the point to the closest edge...
Here is the output from one such failed click on the try build:

02-20 09:31:04.640 I/InputDispatcher(11033): Delivering touch to current input target
02-20 09:31:04.960 E/GeckoConsole(26161): TEST: Initial click 252, 135: A# (233.5,126 10.5x15)
02-20 09:31:04.960 E/GeckoConsole(26161): TEST: In bounds X false (252 > 233.5 && 252 < 244
02-20 09:31:04.970 E/GeckoConsole(26161): TEST: In bounds Y false (135 > 126 && 135 < 141
02-20 09:31:04.970 E/GeckoConsole(26161): TEST: Moving X min(244, Math.max(234, 252))
02-20 09:31:04.970 E/GeckoConsole(26161): TEST: Moving Y min(141, Math.max(126, 135))
02-20 09:31:04.970 E/GeckoConsole(26161): TEST: Moved to 244, 135: DIV#bug968991 (10,123 237x682)
02-20 09:31:06.520 I/LOG_TAG (11095): [PNN] [getAllEonsNames] OPL_MCCMNC[0]=[302490]
Weird. Moving the point closer seems to cause anyElementFromPoint to return something different. What's the markup look like?
Attached file Sample page
The page has changed since I provided the log, but it has the same general format. The link I was trying to click is one of the "X" links in the top bar of the bug block.
tracking-fennec: ? → 30+
Attached patch PatchSplinter Review
Hm.... so the problem is after we correct the point to be "inside" the element, we do one last anyElementFromPoint call to ensure that there isn't actually something else (on top) at that point that the click should hit. i.e. I imagined an image inside an anchor and wanted to make sure the event bubbled correctly. In this case that seems to find the wrong element. The div underneath the anchor rather than the anchor itself.

I wonder if we're doing advanced hit detection and taking alpha into account, but the platform code doesn't seem to indicate that. This particular anchor has some margin around it? Maybe the hit detection code find a different frame for that and walks up the tree to the wrong node?

We can remove that call with little effect (I think). Otherwise we could ping roc or someone for details about elementFromPoint. Build at 
people.mozilla.com/~wjohnston/touches.apk
Attachment #8383402 - Flags: review?(bugmail.mozilla)
Comment on attachment 8383402 [details] [diff] [review]
Patch

Review of attachment 8383402 [details] [diff] [review]:
-----------------------------------------------------------------

Worth trying, I think.

r=me with commit message fixed.
Attachment #8383402 - Flags: review?(bugmail.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/ad1574c13ebf
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
I'm still seeing this problem. In some cases it seems worse, even. I can run a try build with logging again if you want.
Depends on: 985867
No longer depends on: 985867
You need to log in before you can comment on or make changes to this bug.