Skip to content

Update targets of predicted and coalesced events when trusted event target changes.#390

Merged
patrickhlauke merged 2 commits into
w3c:gh-pagesfrom
flackr:flackr-update-targets
Jul 7, 2021
Merged

Update targets of predicted and coalesced events when trusted event target changes.#390
patrickhlauke merged 2 commits into
w3c:gh-pagesfrom
flackr:flackr-update-targets

Conversation

@flackr

@flackr flackr commented Jul 6, 2021

Copy link
Copy Markdown
Contributor

This allows removing the dirty flag, as trusted events keep the target of the PointerEvent in sync with their coalesced and predicted events.


Preview | Diff

This is also only done for trusted events, establishing the precedent
that for untrusted events there is no required association between the
predicted and coaleseced events and their owning event.
@flackr flackr requested a review from patrickhlauke July 6, 2021 17:26
Comment thread index.html Outdated
<code>isPrimary</code> and <code>isTrusted</code> to the {{PointerEvent}}'s
<code>pointerId</code>, <code>pointerType</code>, <code>isPrimary</code> and
<code>isTrusted</code>.</li>
<code>pointerId</code>, <code>pointerType</code>, <code>target</code>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how precise we want to be here. When a PointerEvent is created, it doesn't have a target at all. target is set during event dispatch. https://dom.spec.whatwg.org/#concept-event-listener-invoke

Comment thread index.html
and <a><code>predicted events targets dirty</code></a> to true.</p>

<p>When the <code><a data-lt="PointerEvent.getCoalescedEvents">getCoalescedEvents</a></code> method is called:</p>
<p>When a trusted <a>PointerEvent</a>'s target is changed:</p>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...in fact, isn't this enough. target changes when event is dispatched.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this is much simpler. Updated.

@patrickhlauke

Copy link
Copy Markdown
Member

assume this can then be grafted back in/onto #377

going to trust @smaug---- on the whether or not this sufficiently addresses our concern there

Comment thread index.html
<li>If the <a>coalesced events targets dirty</a> is true:
for each event in the <a>coalesced event list</a>, set the event's {{Event/target}}
<li>for each event in the <a>coalesced event list</a>, set the event's {{Event/target}}
to this <code>PointerEvent</code>'s target.</li>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: target instead of plain "target"?

Comment thread index.html
<li>If the <a>coalesced events targets dirty</a> is true:
for each event in the <a>coalesced event list</a>, set the event's {{Event/target}}
<li>for each event in the <a>coalesced event list</a>, set the event's {{Event/target}}
to this <code>PointerEvent</code>'s target.</li>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was there before but never came to me: the "this" here seems unclear to me. The first point in the above para (source line 1110) doesn't use "this", but also unclear to me :( .

  • Can we make both paras consistent?
  • Can we perhaps use something like "container event"?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep in mind that this PR will then be followed by #377 which completely rejigs this stuff....

@mustaqahmed mustaqahmed self-requested a review July 7, 2021 15:10
@patrickhlauke patrickhlauke merged commit 4bd6e8b into w3c:gh-pages Jul 7, 2021
@smaug---- smaug---- added needs-wpt Investigation whether the issue needs a wpt test has been done and wpt is missing and removed wpt labels Nov 23, 2022
@smaug----

Copy link
Copy Markdown
Contributor

I couldn't find a test for this

@smaug----

Copy link
Copy Markdown
Contributor

I'm writing a test for this.

@smaug---- smaug---- self-assigned this Oct 11, 2023
@smaug---- smaug---- removed the needs-wpt Investigation whether the issue needs a wpt test has been done and wpt is missing label May 27, 2024
@smaug----

Copy link
Copy Markdown
Contributor

A super simple manual wpt is here web-platform-tests/wpt#46485
It needs to be manual, since testing predicted events isn't really possibly through non-manual WPTs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants