Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify what event is userEvent to consider click/auxclick event target after touchend (or pointerup) #508

Open
masayuki-nakano opened this issue Jun 18, 2024 · 15 comments

Comments

@masayuki-nakano
Copy link

Pointer Events defines that click/auxclick event target is:

  1. If userEvent is not a PointerEvent, dispatch event following the [UIEVENTS] spec without overriding event target and skip the remaining steps below.
  2. If event is a contextmenu event, or userEvent was dispatched while the corresponding pointer was captured, then let target be the target of userEvent.

and userEvent is defined as:

userEvent be the user interaction event that caused the firing of event.

If you touch the device with a single input, touchstart and touchend may cause compatibility mouse events, mousemove, mousedown and mouseup. However, web apps consumes pointerdown or touchstart, the compatibility mouse events are not fired. Therefore, userEvent may be pointerup, touchend or mouseup. The compatibility mouse event target is defined as the element at the touchend position. The touchend event target is defined as the element which is the target of touchstart. So, these events may be targeted different elements. Therefore, click/auxclick event target may be different from which event is userEvent.

@masayuki-nakano
Copy link
Author

Oh, if touchstart is canceled, click should not be fired.

@masayuki-nakano
Copy link
Author

Created a new test which (probably) conforms to current event specs. However, as I mentioned in the commit message, conforming to the test result may break bxSlider.

@masayuki-nakano
Copy link
Author

Ccing: @mustaqahmed, @smaug----, @flackr

@masayuki-nakano
Copy link
Author

(FYI: This issue blocks Mozilla fixing the failures of pointerevent_click_during_capture.html.)

@masayuki-nakano
Copy link
Author

Currently, if I change Firefox behavior to conform to the spec, links in this bxSlider deom works, but the originally reported example has gone. Therefore, I don't know whether bxSlider has fixed the issue actually.

@masayuki-nakano
Copy link
Author

Sorting out the issues:

  • click event target may be different even if same position is clicked/tapped, depending on pointerType is mouse or touch (see the comments in my testcase)
  • some developers might expect that capturing parent event target of pointerdown won't stop dispatching click on the original target (but the reported web site seems working with current defined behavior today)

At least the former breaks a Pointer Event purpose to handle all pointing device inputs with one set of DOM events.

@flackr
Copy link
Contributor

flackr commented Jun 18, 2024

The note in that section implies (at least to me) that the userEvent is the PointerEvent (i.e. pointerup) whose default action caused the dispatch (excepting keyboard / accessibility means of dispatching clicks):

Event userEvent could be a non-PointerEvent; for example, it is a KeyboardEvent when a click event dispatch is caused by hitting the spacebar on a checkbox element.

When userEvent is a PointerEvent, userEvent is a pointerup for a click or auxclick event, and either a pointerdown or a pointerup event (depending on native platform convention) for a contextmenu event.

But I'm not sure I understand the specific differences that are causing problems. Most pages won't be setting pointerCapture so the target of the click event will be the same as it usually is.

@masayuki-nakano
Copy link
Author

The note in that section implies (at least to me) that the userEvent is the PointerEvent (i.e. pointerup) whose default action caused the dispatch (excepting keyboard / accessibility means of dispatching clicks):

If so, the click event target won't depend on mouse or touch, but both Chrome and Firefox dispatch it on wrong target, they dispatch click on the preceding touchend target which is same as touchstart's.

@flackr
Copy link
Contributor

flackr commented Jun 19, 2024

The note in that section implies (at least to me) that the userEvent is the PointerEvent (i.e. pointerup) whose default action caused the dispatch (excepting keyboard / accessibility means of dispatching clicks):

If so, the click event target won't depend on mouse or touch, but both Chrome and Firefox dispatch it on wrong target, they dispatch click on the preceding touchend target which is same as touchstart's.

Is this not the same as the pointerup target? Note that touch usually has implicit capture so these would all be the same target AFAIK.

@masayuki-nakano
Copy link
Author

The note in that section implies (at least to me) that the userEvent is the PointerEvent (i.e. pointerup) whose default action caused the dispatch (excepting keyboard / accessibility means of dispatching clicks):

If so, the click event target won't depend on mouse or touch, but both Chrome and Firefox dispatch it on wrong target, they dispatch click on the preceding touchend target which is same as touchstart's.

Is this not the same as the pointerup target? Note that touch usually has implicit capture so these would all be the same target AFAIK.

No if pointerdown listener makes different element from pointerdown/touchstart/touchend target capture the pointer. The expected result of line 192 of my test passed on Chrome and Firefox.

@flackr
Copy link
Contributor

flackr commented Jun 19, 2024

No if pointerdown listener makes different element from pointerdown/touchstart/touchend target capture the pointer. The expected result of line 192 of my test passed on Chrome and Firefox.

If pointerdown captures the pointer to a different element, pointerup should "target" (via capture) that different element too though.

@masayuki-nakano
Copy link
Author

No if pointerdown listener makes different element from pointerdown/touchstart/touchend target capture the pointer. The expected result of line 192 of my test passed on Chrome and Firefox.

If pointerdown captures the pointer to a different element, pointerup should "target" (via capture) that different element too though.

Yeah, however, both Chrome and Firefox dispatch click event on the #target instead of #parent which is captured and is the target of pointerup. So, both browsers treat userEvent as the preceding touchend or mouseup.

@flackr
Copy link
Contributor

flackr commented Jun 20, 2024

Yeah, however, both Chrome and Firefox dispatch click event on the #target instead of #parent which is captured and is the target of pointerup. So, both browsers treat userEvent as the preceding touchend or mouseup.

I believe if you try chrome with the --enable-blink-features=BoundaryEventDispatchTracksNodeRemoval flag that it will dispatch to the captured target of the pointerevent instead (code here). @mustaqahmed I'm not sure why we have this change behind that flag, I thought this was a separate issue.

@masayuki-nakano
Copy link
Author

I believe if you try chrome with the --enable-blink-features=BoundaryEventDispatchTracksNodeRemoval flag that it will dispatch to the captured target of the pointerevent instead (code here).

Well, I assume that it's enabled by default when I run WPT on Chrome from ./mach wpt command in Firefox development environment (I use Chrome Canary). The command does not accept to explicitly enable it, but in the same condition, all tests of pointerevent_after_target_removed.html?mouse pass so I believe that the feature is enabled by default when I run Chrome Canary from the command. However, my test unexpectedly pass on Chrome Canary.

When I run Chrome Canary, I see the following warning in the terminal:

 0:14.97 pid:56536 [4464:22700:0623/115359.500:WARNING:chrome_browser_main_win.cc(712)] Command line too long for RegisterApplicationRestart:  --allow-pre-commit-input --autoplay-policy=no-user-gesture-required --disable-background-networking --disable-backgrounding-occluded-windows --disable-client-side-phishing-detection --disable-default-apps --disable-hang-monitor --disable-infobars --disable-popup-blocking --disable-prompt-on-repost --disable-sync --enable-blink-features=DisableAhemAntialias --enable-experimental-web-platform-features --enable-features=FedCmWithoutWellKnownEnforcement,SecurePaymentConfirmationBrowser,GenericSensorExtraClasses --enable-logging=stderr --headless=new --host-resolver-rules="MAP nonexistent.*.test ^NOTFOUND, MAP *.test 127.0.0.1, MAP *.test. 127.0.0.1" --ignore-certificate-errors-spki-list=sCJ8962Wxqgz44IKoPQLcDT7YRRAxO2w1iYIqpMYHhg=,0Rt4mT6SJXojEMHTnKnlJ/hBKMBcI4kteBlhR1eTTdk= --ip-address-space-overrides=127.0.0.1:8002=private,127.0.0.1:8003=public,127.0.0.1:8445=private,127.0.0.1:8446=public --log-level=0 --no-first-run --no-service-autorun --noerrdialogs --password-store=basic --remote-debugging-port=0 --short-reporting-delay --test-type=webdriver --use-fake-device-for-media-stream --use-fake-ui-for-digital-identity --use-fake-ui-for-fedcm --use-fake-ui-for-media-stream --use-mock-keychain --user-data-dir="C:\Temp\scoped_dir56536_1771468703" --webtransport-developer-mode --restore-last-session --restart

Doesn't --enable-experimental-web-platform-features enable the feature as well?

@flackr
Copy link
Contributor

flackr commented Jun 24, 2024

Doesn't --enable-experimental-web-platform-features enable the feature as well?

Ah, yes, it does. That turns on all features with status: experimental in runtime_enabled_features.json5 which it turns out BoundaryEventDispatchTracksNodeRemoval is.

However, my test unexpectedly pass on Chrome Canary.

@mustaqahmed could you look into this test case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants