1473108 - Link child's ::before and :active::after content can prevent part of the link from reacting to taps (but clicks work).

Status: VERIFIED FIXED

(Core :: DOM: Events, defect, P1)

Description

[Thomas Wisniewski [:twisniewski]]

Attachment: test.html

The provided test-case has a single link (green) which is partly covered by up by the ::before content of a span inside of it (red overlay). This content is normally tappable and clickable.

However, when that span also has an :active::after rule applied to it, suddenly tapping no longer registers on the parts of the link which are obscured by the span's ::before or ::after content. Clicks still react as expected.

I have not tested with a desktop touchscreen, only on a phone running Fennec. As such this may be Fennec-specific. Chrome on the same phone reacts to taps on the red region as well.

Comment 1

[Thomas Wisniewski [:twisniewski]]

Also note that this affects Google Tier1 interop.

Comment 2

[Andrew McCreight [:mccr8]]

kats, do you have any ideas here? I don't know if this is likely to be APZ, layout, DOM, or something else. Thanks.

I'll go ahead and mark this P1 based on comment 1.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

So when I reproduce this problem it seems like every other tap on the red part works. That's pretty weird. Without further investigation it could be a bug any of APZ, layout, or DOM. I'll do a little digging, leaving needinfo on me for now.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Logging with frame dumps and event targets

I turned on/modified some logging in APZEventState.cpp and PositionedEventTargeting.cpp to see what was happening with the event. As far as I can tell the APZ code is working fine - it turns the touch tap into a click, and dispatches the mousemove, mousedown, mouseup events correctly. The target frame for all of these events in PositionedEventTargeting is the Block(_moz_generated_content_before) frame, which is correct. So as far as I can tell the APZ and layout parts of this are fine. So DOM:Events seems like the right place for this to be.

Comment 5

[Andrew McCreight [:mccr8]]

Thanks for taking a look.

Olli, do you know who might be able to investigate this tap event problem on Google at the DOM level?

Comment 6

[Emilio Cobos Álvarez (:emilio)]

I'm 99% sure this is because the :active selector causes the ::after content to reframe the event target.

So, even though this should probably be fixed in DOM: Events, we could make layout reframe in a more fine-grained way to avoid causing the issue in the first place. This has the same root cause as bug 1448813.

Comment 7

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)
> I'm 99% sure this is because the :active selector causes the ::after content
> to reframe the event target.
> 
> So, even though this should probably be fixed in DOM: Events, we could make
> layout reframe in a more fine-grained way to avoid causing the issue in the
> first place. This has the same root cause as bug 1448813.

I guess this is why EventStateManager does not generate MouseClick event for the :active::after content [1].

[1] https://searchfox.org/mozilla-central/rev/c296d5b2391c8b37374b118180b64cca66c0aa16/dom/events/EventStateManager.cpp#5049-5051

Comment 8

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

(In reply to Edgar Chen [:edgar] from comment #7)
> EventStateManager does not generate MouseClick event for
> the :active::after content [1].
> 
> [1] https://searchfox.org/mozilla-central/rev/c296d5b2391c8b37374b118180b64cca66c0aa16/dom/events/EventStateManager.cpp#5049-5051

EventStateManager could not get the event content [2] because of the UnbindFromTree in nsBlockFrame::DoRemoveFrame [3].

[2] https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/dom/events/EventStateManager.cpp#5017
[3] https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/layout/generic/nsBlockFrame.h#538

Comment 9

[Emilio Cobos Álvarez (:emilio)]

I just pointed Edgar to bug 1489139 / bug 1461299 which are pretty related.

ESM should be able to do similar cleanup to point the target to the non-anonymous element if needed.

Comment 10

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1&revision=1c4be46478fa9b38b25e68c5816c7624bdfd1266&group_state=expanded

Comment 11

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1&revision=fcdcb94d2fdcf1b7e511a7a9b23c23a09fb04dc7&group_state=expanded

Comment 12

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Bug 1473108 - Part 1: Devirtualize nsIPresShell::GetCurrentEventFrame/GetEventTargetContent;

Comment 13

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Bug 1473108 - Part 2: Make PresShell not point to unbound NAC in event content stack;

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b38223cb4fd1
Part 1: Devirtualize nsIPresShell::GetCurrentEventFrame/GetEventTargetContent; r=smaug
https://hg.mozilla.org/integration/autoland/rev/ecce651fad51
Part 2: Make PresShell not point to unbound NAC in event content stack; r=smaug

Comment 15

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/b38223cb4fd1
https://hg.mozilla.org/mozilla-central/rev/ecce651fad51

Comment 16

[Bogdan Surd, QA [:bsurd, NI]]

Device:
 - Samsung Galaxy Note 8 (Android 8.0)

Verified in the latest nightly using the test case provided in Comment 0 as well as tapping on the google account icon as described in the github issue. Everything works as expected.

Comment 17

[Thomas Wisniewski [:twisniewski]]

Unfortunately I just tested the tier-1 Google site icon on today's Fennec nightly (with a spoofed Chrome UA), and it still isn't working properly. The test-case is, however. So it appears that there is likely still some other interop issue at play. I'll investigate and file a new bug as appropriate.