1792110 - Difference between Firefox and Chrome event propagation when using pointer-events: none on :active pseudo selector

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Ksenia Berezina [:ksenia]]

Attachment: 108202.html

This was initially reported in https://github.com/webcompat/web-bugs/issues/108202 where user is unable to click on certain buttons to share, search, open menus, etc. on stocktwits.com new design.

Looks like this is happening because parents of those buttons have pointer-events: none;, but only for the :active state:

.Dropdown_triggerWrap__LW6BW:active {
  pointer-events: none;
}

This is preventing event from bubbling to this element's parent, and therefore the event handler is not triggered, but only in Firefox. Event propagation works in Chrome and the site seems to be relying on that.

I've attached a reduced test case and was wondering whether Firefox behavior is correct here?

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Edgar, do you have thoughts about the expected behavior here?

Also wondering - is this is something in interop investigation list?

Comment 2

[Edgar Chen [:edgar]]

(In reply to Hsin-Yi Tsai (she/her) [:hsinyi] from comment #1)

Edgar, do you have thoughts about the expected behavior here?

In bug 1089326, we change the click handling to follow blink model (which is closer to the spec) which fires click event on the closest common ancestor of mousedown/mouseup targets. If we change the <button> to some other elements, e.g. <a> or <div>, in the test script, we behave the same as blink, i.e. click event is fired. I am not sure why we behave different on <button>, maybe just a bug since I don't see a reason that we need to behave differently on <button>.

Also wondering - is this is something in interop investigation list?

No, and this issue is more about how we generate click event, not related to pointer events.

Comment 3

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar] from comment #2)

If we change the <button> to some other elements, e.g. <a> or <div>, in the test script, we behave the same as blink, i.e. click event is fired.

Err, it doesn't work if it is <a> element, I think it is because we stop finding the common ancestor at the interactive element in https://searchfox.org/mozilla-central/rev/91ed81b76015e49ebd55a6de5df2b034456c15e8/dom/base/nsContentUtils.cpp#2896-2941. :smaug, is there any specific reason doing this, I could not find spec mention this (maybe spec has changed at some point?).

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

The reason for that code in nsContentUtils is that it was the behavior Chrome had at that time. I believe they then changed the behavior.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #4)

The reason for that code in nsContentUtils is that it was the behavior Chrome had at that time. I believe they then changed the behavior.

So... which is the behavior we'd like to keep for now?

Comment 6

[Sean Feng [:sefeng]]

This looks like a Firefox bug to me. I checked the spec for pointer-event: none, I don't think we want to stop the event bubbling to ancestors intentionally. The spec explicitly mentions the event should still propagate to descendants, so presumably this applies ancestors as well. The provided testcase works in Chrome and Safari, so we should fix this I think.

Unless anyone disagree, I am needing info myself to make a patch.

Comment 7

[Sean Feng [:sefeng]]

Attachment: Bug 1792110 - Update the common ancestor finding logic for mouseup with mousedown r=edgar

The current behavior in Firefox doesn't match to what Chrome and
Safari do. Given the spec doesn't mention anything about stop bubbling
to ancestors, and the spec mentions the event should still propagate to
descendants, I think this is the correct behavior.

Comment 8

[Sean Feng [:sefeng]]

Attachment: Bug 1792110 - Updathe PIP tests to align with the latest change for dispatching click events r=edgar,niklas

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/37614918cfa8
Update the common ancestor finding logic for mouseup with mousedown r=edgar
https://hg.mozilla.org/integration/autoland/rev/db71444cb84f
Updathe PIP tests to align with the latest change for dispatching click events r=edgar,niklas

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40460 for changes under testing/web-platform/tests

Comment 11

[Narcis Beleuzu [:NarcisB]]

Backed out for mochitest failure on test_bug1089326.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/002eed48258e10f0410fd6f2db87d894227e122b
Log link: https://treeherder.mozilla.org/logviewer?job_id=418615440&repo=autoland&lineNumber=2019

Comment 12

[Sean Feng [:sefeng]]

Attachment: Bug 1792110 - Update test_Bug 1792110.html to accommodate the common ancestor changes for mouseup r=edgar

Comment 13

[Narcis Beleuzu [:NarcisB]]

Please also check :

• this wpt failure on click_event_target_child_parent.html
• this mochitest crash

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63bffcdc61e8
Update the common ancestor finding logic for mouseup with mousedown r=edgar
https://hg.mozilla.org/integration/autoland/rev/083b3dc773f3
Updathe PIP tests to align with the latest change for dispatching click events r=edgar,niklas
https://hg.mozilla.org/integration/autoland/rev/0d88c6fe314b
Update test_Bug 1792110.html to accommodate the common ancestor changes for mouseup r=edgar

Comment 16

[Norisz Fay [:noriszfay]]

Backed out for causing mochitest failures on test_bug756984.html

Backout link

Push with failures

Failure log

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:sefeng, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 19

[Sean Feng [:sefeng]]

I am updating the patch for some additional changes.

Comment 20

[Sean Feng [:sefeng]]

Attachment: Bug 1792110 - Make no common ancestor between mouseup and mousedown when the input control type is different r=edgar

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d492bd7d93d7
Update the common ancestor finding logic for mouseup with mousedown r=edgar
https://hg.mozilla.org/integration/autoland/rev/9cfe7f3b3e3e
Updathe PIP tests to align with the latest change for dispatching click events r=edgar,niklas
https://hg.mozilla.org/integration/autoland/rev/c29961741ee1
Update test_Bug 1792110.html to accommodate the common ancestor changes for mouseup r=edgar
https://hg.mozilla.org/integration/autoland/rev/510ddee05cfc
Make no common ancestor between mouseup and mousedown when the input control type is different r=edgar
https://hg.mozilla.org/integration/autoland/rev/66ee3cdf67e0
apply code formatting via Lando

Comment 22

[Norisz Fay [:noriszfay]]

Backed out for causing ThreadSanitizer related failures and failures on test_bug756984.html

Backout link

Push with failures - 9
Push with failures - 11

Failure log - 9
Failure log - 11

There's also this failure present.

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f93a68453710
Update the common ancestor finding logic for mouseup with mousedown r=edgar
https://hg.mozilla.org/integration/autoland/rev/96e3aa655e92
Updathe PIP tests to align with the latest change for dispatching click events r=edgar,niklas
https://hg.mozilla.org/integration/autoland/rev/bf859c9e86fd
Update test_Bug 1792110.html to accommodate the common ancestor changes for mouseup r=edgar
https://hg.mozilla.org/integration/autoland/rev/92ad9c748611
Make no common ancestor between mouseup and mousedown when the input control type is different r=edgar

Comment 25

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/f93a68453710
https://hg.mozilla.org/mozilla-central/rev/96e3aa655e92
https://hg.mozilla.org/mozilla-central/rev/bf859c9e86fd
https://hg.mozilla.org/mozilla-central/rev/92ad9c748611

Comment 26

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 27

[Sean Feng [:sefeng]]

Ryan, do you mind back this patch out from Beta, so that we have more time to fix the regressions? Thanks!

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Backed out for 117.0b9. This change remains in place for 118+. Per discussion with Sean, the fix for bug 1846714 is safe to leave in place even with this backed out, so I didn't include that.
https://hg.mozilla.org/releases/mozilla-beta/rev/f39831155fa1