-
Notifications
You must be signed in to change notification settings - Fork 295
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
Compute everything needed for composedPath() during dispatch #535
Conversation
@tabatkins I get
but I didn't even touch that line in this commit (nor does it contain |
@hayatoito @smaug---- while the Preview link in OP is broken, the Diff link does somehow work and shows you the changes rendered in HTML. Hope that helps. |
dom.bs
Outdated
|
||
<li><p>Let <var>isActivationEvent</var> be true, if <var>event</var> is a {{MouseEvent}} object and | ||
<var>event</var>'s {{Event/type}} attribute is "<code>click</code>", and false otherwise. | ||
|
||
<li><p>Let <var>activationTarget</var> be <var>target</var>, if <var>isActivationEvent</var> is | ||
true and <var>target</var> has <a for=EventTarget>activation behavior</a>, and null otherwise. | ||
|
||
<li><p>Let <var>slotable</var> be null. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should set slotable to target if target is assigned, as we are doing in 11.2, here too.
Ugh, I have a fix for the line numbers pointing wrong on my laptop (it's caused by me badly handling comment-stripping). It's trying to point to one of the lines in the early-600s, because you're using a 1-space indent but haven't told Bikeshed that (it defaults to assuming a 4-space indent for markdown purposes - this includes html). |
And fix is finished and pushed - it now correctly points out the problem is on line 631. You need to use a consistent indentation level thruout your spec - either set |
@hayatoito @smaug---- are either of you planning on writing tests covering all the various cases discussed? |
Yeah, let me add WPT on this. |
Add a test for the change of DOM Standard. See whatwg/dom#535 for details. Change-Id: I725fc9e08ca72b5e1e083a67276decf0cdd8bfc8
Add a test for the change of DOM Standard. See whatwg/dom#535 for details. Change-Id: I725fc9e08ca72b5e1e083a67276decf0cdd8bfc8 Reviewed-on: https://chromium-review.googlesource.com/781303 Reviewed-by: Takayoshi Kochi <kochi@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#518178}
Add a test for the change of DOM Standard. See whatwg/dom#535 for details. Change-Id: I725fc9e08ca72b5e1e083a67276decf0cdd8bfc8 Reviewed-on: https://chromium-review.googlesource.com/781303 Reviewed-by: Takayoshi Kochi <kochi@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#518178}
The test was added at: web-platform-tests/wpt#8385 |
Thank you! |
Add a test for the change of DOM Standard. See whatwg/dom#535 for details. Change-Id: I725fc9e08ca72b5e1e083a67276decf0cdd8bfc8 Reviewed-on: https://chromium-review.googlesource.com/781303 Reviewed-by: Takayoshi Kochi <kochi@chromium.org> Commit-Queue: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#518178}
The existing algorithm exposed nodes in shadow trees that should remain hidden. (This wasn't noticed in #535.) This roughly matches the approach taken by jsdom to solve this issue and unlike #696 requires no downstream changes. Tests: shadow-dom/event-composed-path.html in wpt. Fixes #684. Closes #696.
Fixes #525.
Preview | Diff