1966569 - Unexpected navigation when tapping to add a project tag in Phabricator

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[Daniel Holbert [:dholbert]]

Note: this reproduces on Android as well as in RDM. I'm including RDM steps since that's easier for most folks to test.

The tl;dr steps are: try to add a project-tag (e.g. testing-*) to a phab revision on Android or RDM. More detail below.

STR:

1. Be signed into phabricator
2. visit some phab revision like e.g. https://phabricator.services.mozilla.com/D249234
3. Ctrl+Shift+M to enter Responsive Design Mode.
4. If the device-selection dropdown is showing "Responsive", choose an actual device instead (e.g. Galaxy Note 20) and reload the page.
5. Scroll to the bottom of the page and click "Add Action" | "Change Project Tags"
6. Click the Project Tags section and type "te"
7. In the autocompleted dropdown list that appears, choose any of the options (e.g. "testing-approved")

ACTUAL RESULTS:
The tag gets added to the project tags field, but then shortly afterwards, you get navigated to https://phabricator.services.mozilla.com/project/view/133/

EXPECTED RESULTS:
No navigation.

mozregression narrowed this to a pref-flip here:
https://hg-edge.mozilla.org/integration/autoland/pushloghtml?fromchange=fc63623a76df65764289dce018b0f4d818ead34e&tochange=8c31644eed164a0461d14198945b08b039899c30

I confirmed that the issue goes away if I toggle that pref dom.w3c_pointer_events.dispatch_click_on_pointer_capturing_element to false.

--> regression from bug 1885232

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screencast of bug

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, I cannot keep opening the autocomplete popup, therefore, it's hard to understand what's going on from the content. However, this is reproducible with RDM (thank you for checking that!), so, I could debug from C++ side.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm... The server is too slow... I'll take a look tomorrow.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

If event is a contextmenu event, or userEvent was dispatched while the corresponding pointer was captured, then let target be the target of userEvent.

The click event target is defined as this, event is click and userEvent is the preceding pointerup. So, click event target should be the target of pointerup.

However, Chromium implements this differently.

1. Dispatch pointerup
2. Consider to dispatch a click
3. Considers the click event target with capturing element after pointerup
4. Release pointer capture implicitly
5. Process the pending pointer capture
6. Dispatch click on the captured element

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

It seems that Phabricator listens to mousedown and hide the autocomplete popup. Therefore, if the click target is not the ex-capture element, the hit-testing at dispatching pointerup won't reach the <div> which is a child of <a>.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oh, according to the testcase, Chrome does not use the pointerup target for click if it's caused by a touch. I'm looking for the dispatcher which must be different point from comment 4.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1966569 - Stop using the standardized `click` target if it's caused by a tap r=smaug!

Pointer Events spec implies that the source event of click is the preceding
pointerup unless it's caused by keyboard or script and that defines the
click event target should be the pointerup event target if the pointer is
captured either explicitly or implicitly. However, when it's caused by a tap,
Chrome does not use the pointerup target as the click event target. For
now, we should align the behavior to Chrome because we got a web-compat issue on
Phabricator.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/66a2dfa4a803
Stop using the standardized `click` target if it's caused by a tap r=smaug

Comment 9

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/66a2dfa4a803