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

Coalesced and predicted event attributes within an untrusted event #514

Closed
mustaqahmed opened this issue Aug 28, 2024 · 10 comments · Fixed by #524
Closed

Coalesced and predicted event attributes within an untrusted event #514

mustaqahmed opened this issue Aug 28, 2024 · 10 comments · Fixed by #524
Assignees

Comments

@mustaqahmed
Copy link
Member

The spec Sec 10 mentions the following about the attributes of the "contained events" for a JS-constructed (untrusted) event, which needs a clarification for at least isTrusted and target attributes:

Untrusted events have their coalesced/predicted events list initialized to the value passed to the constructor.

This text seems to suggest that the constructor allows setting any arbitrary value to an attribute. However, when I run the following code in Chrome and Firefox:

// Read default values:
let e0 = new PointerEvent("pointermove");
console.log(e0.isTrusted, e0.bubbles, e0.clientX, e0.pointerType, e0.target);
// Read assigned values:
let e1 = new PointerEvent("pointermove", {isTrusted: true, bubbles: true, clientX: 1234, pointerType: "hand", target: document.body});
console.log(e1.isTrusted, e1.bubbles, e1.clientX, e1.pointerType, e1.target);

both browsers emit the following:

false false 0 '' null
false true 1234 'hand' null

So isTrusted and target attributes do not allow arbitrary values while other attributes allow. I didn't see any indication in IDLs to explain this behavior!

In any case, we need clarifications for those two special attributes in coalesced and predicted event lists:

  • Within a JS-constructed "parent" event, will they always have their default values?
  • When a trusted "parent" event is redispatched from JS, will they be reset to their default values?
@mustaqahmed
Copy link
Member Author

FYI @smaug---- @flackr

@smaug----
Copy link
Contributor

There is no isTrusted property in the *EventInit, and there is no target.

I'm confused about this issue. The sentence in the spec talks about how one can initialize coalesced and predicted events lists. PointerEventInit has properties for those.

JS created event is always non-trusted, so redispatching doesn't affect .target of coalesced/predicted events.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 3, 2024
The test is tentative due to:
w3c/pointerevents#514

Bug: 353538500
Change-Id: Id3ead2029e84a3100573558adbd7c14c272e821f
@mustaqahmed
Copy link
Member Author

mustaqahmed commented Sep 4, 2024

Thanks @smaug----, that answers my first question.

My second question remains. To clarify, this is about JS re-dispatch of browser-constructed event. Something like this with the user moving the mouse over target2:

target1.onpointermove = e => console.log(e.getCoalescedEvents()[0].isTrusted);
target2.onpointermove = e => setTimeout(() => target1.dispatchEvent(e), 0);

(I am landing a tentative WPT for this.)

@mustaqahmed
Copy link
Member Author

FYI: both Firefox and Chromium emit true from the repro I just posted, and I found it questionable.

@smaug----
Copy link
Contributor

Oh, the "parent" event has been dispatched once, and then you redispatch it again? I wouldn't expect anything to change for coalesced or predicted events

@flackr
Copy link
Contributor

flackr commented Sep 4, 2024

I'm remembering now that previously we decided these additional structures would be treated as completely generic objects for developer dispatched events. This makes sense since the user could put anything they want in the coalescedEvents / predictedEvents lists, and I guess as such developers shouldn't trust the isTrusted flag or target attribute on those events if the isTrusted on the root event is false.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 4, 2024
The test is tentative due to:
w3c/pointerevents#514

Bug: 353538500
Change-Id: Id3ead2029e84a3100573558adbd7c14c272e821f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5823713
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1350816}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 4, 2024
The test is tentative due to:
w3c/pointerevents#514

Bug: 353538500
Change-Id: Id3ead2029e84a3100573558adbd7c14c272e821f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5823713
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1350816}
@mustaqahmed
Copy link
Member Author

Well, the JS seems unable to construct a new PointerEvent object with e.isTrusted == true, and can't even change the same attribute in an existing PointerEvent object (trusted or not). Maybe this is because of the IDL for isTrusted, not sure:

[LegacyUnforgeable] readonly attribute boolean isTrusted;

Did I miss anything? At least the spec needs a clarification, right?

@smaug----
Copy link
Contributor

That is the whole point of isTrusted. It is set to true only for events which UA dispatches. If an event was trusted and it is later re-dispatched using JS, it becomes untrusted.

@mustaqahmed
Copy link
Member Author

Exactly! Therefore the current behavior essentially says "the isTrusted bits inside coalesced/predicted lists are always set by the browser yet they don't reflect if the events are really trusted or not". I find it super-confusing for developers.

If we want to maintain this behavior, I would suggest adding a spec note accordingly.

@smaug----
Copy link
Contributor

smaug---- commented Sep 4, 2024

The current behavior says that on untrusted events native code doesn't change anything on the coalesced/predicted lists, no?

(but of course we should clarify the spec, if there is something unclear)

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 6, 2024
… on redispatch., a=testonly

Automatic update from web-platform-tests
Add a WPT for coalesced event attributes on redispatch.

The test is tentative due to:
w3c/pointerevents#514

Bug: 353538500
Change-Id: Id3ead2029e84a3100573558adbd7c14c272e821f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5823713
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1350816}

--

wpt-commits: ab72b1d1516a5f8ed2a88af3e9a4fec1817c3a84
wpt-pr: 47940
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Sep 6, 2024
… on redispatch., a=testonly

Automatic update from web-platform-tests
Add a WPT for coalesced event attributes on redispatch.

The test is tentative due to:
w3c/pointerevents#514

Bug: 353538500
Change-Id: Id3ead2029e84a3100573558adbd7c14c272e821f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5823713
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1350816}

--

wpt-commits: ab72b1d1516a5f8ed2a88af3e9a4fec1817c3a84
wpt-pr: 47940
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Sep 7, 2024
… on redispatch., a=testonly

Automatic update from web-platform-tests
Add a WPT for coalesced event attributes on redispatch.

The test is tentative due to:
w3c/pointerevents#514

Bug: 353538500
Change-Id: Id3ead2029e84a3100573558adbd7c14c272e821f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5823713
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1350816}

--

wpt-commits: ab72b1d1516a5f8ed2a88af3e9a4fec1817c3a84
wpt-pr: 47940
@mustaqahmed mustaqahmed self-assigned this Oct 9, 2024
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 23, 2024
The spec issue has been resolved:
w3c/pointerevents#514

Change-Id: I453fc30258e6b599d65330a63b02822b6815e6fb
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 23, 2024
The spec issue has been resolved:
w3c/pointerevents#514

Change-Id: I453fc30258e6b599d65330a63b02822b6815e6fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5957237
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1372866}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 23, 2024
The spec issue has been resolved:
w3c/pointerevents#514

Change-Id: I453fc30258e6b599d65330a63b02822b6815e6fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5957237
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1372866}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 23, 2024
The spec issue has been resolved:
w3c/pointerevents#514

Change-Id: I453fc30258e6b599d65330a63b02822b6815e6fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5957237
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1372866}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 26, 2024
… non-tentative., a=testonly

Automatic update from web-platform-tests
Mark a coalesced event attribute test as non-tentative.

The spec issue has been resolved:
w3c/pointerevents#514

Change-Id: I453fc30258e6b599d65330a63b02822b6815e6fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5957237
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1372866}

--

wpt-commits: 69ce0d386cba2a63c29d1715e07f22658fdc88b7
wpt-pr: 48779
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Oct 27, 2024
… non-tentative., a=testonly

Automatic update from web-platform-tests
Mark a coalesced event attribute test as non-tentative.

The spec issue has been resolved:
w3c/pointerevents#514

Change-Id: I453fc30258e6b599d65330a63b02822b6815e6fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5957237
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1372866}

--

wpt-commits: 69ce0d386cba2a63c29d1715e07f22658fdc88b7
wpt-pr: 48779
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Oct 30, 2024
… non-tentative., a=testonly

Automatic update from web-platform-tests
Mark a coalesced event attribute test as non-tentative.

The spec issue has been resolved:
w3c/pointerevents#514

Change-Id: I453fc30258e6b599d65330a63b02822b6815e6fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5957237
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1372866}

--

wpt-commits: 69ce0d386cba2a63c29d1715e07f22658fdc88b7
wpt-pr: 48779
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

Successfully merging a pull request may close this issue.

3 participants