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.

Sign up for GitHub

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

Jump to bottom

Manage slot manipulation centrally and special case replace operations #695

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Manage slot manipulation centrally and special case replace operations #695

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 42 additions & 25 deletions dom.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2279,6 +2279,36 @@ steps:
<li><p>If <var>slot</var> is non-null, then run <a>assign slottables</a> for <var>slot</var>.
</ol>

<p>To <dfn noexport>update slots</dfn>, given <var>removedNodes</var>, <var>parent</var>, and
<var>addedNodes</var>, run these steps:

<ol>
<li>
<p>For each <var>removedNode</var> in <var>removedNodes</var>:

<ol>
<li><p>If <var>removedNode</var> is <a for=slotable>assigned</a>, then run
<a>assign slotables</a> for <var>removedNode</var>'s <a>assigned slot</a>.

<li><p>If <var>removedNode</var> has an <a>inclusive descendant</a> that is a <a>slot</a>, then
run <a>assign slotables for a tree</a> with <var>removedNode</var>.
Copy link
Member

@EdgarChen EdgarChen May 3, 2019 •
edited

Choose a reason for hiding this comment

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

I found that it seems the order of slotchange event is changed in a certain case, for example, https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=6882. If I read spec correctly, the event order of current spec is [slot1, slot3. slot2], and it changes to [slot2, slot1, slot3]. Will this order change cause any problem? Current browsers have a different order than spec, and the order is not the same between browsers, either.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! @rniwa do you have any thoughts on order? I looked this through with Edgar and either way we are going to change the order here in order to centralize some of this work for "replace all". I don't care strongly for which slot we signal things first.

And also, given that browsers are already different, it probably does not matter much?

Not sure who to copy from Chrome. @domenic?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm... looks like Blink & Gecko do: [slot1, slot2, slot3] and WebKit does [slot3, slot2, slot1]

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the main point here is that if we want to centralize work and special "replace all" to avoid redundant events, the order will change somewhere, right? So we need to make some kind of call about whether that is okay and whether there are certain principles for deciding on an order. (I think it's okay and although I'd like tree order it doesn't seem worth the additional instructions that'd require.)

</ol>

<li><p>If <var>parent</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a> and
<var>parent</var> is a <a>slot</a> whose <a for=slot>assigned nodes</a> is the empty list, then run
<a>signal a slot change</a> for <var>parent</var>.

<li><p>Run <a>assign slotables for a tree</a> with <var>parent</var>'s <a for=tree>root</a>.

<li>
<p>For each <var>addedNode</var> in <var>addedNodes</var>:

<ol>
<li><p>If <var>parent</var> is a <a for=Element>shadow host</a> and <var>addedNode</var> is a
<a>slotable</a>, then <a>assign a slot</a> for <var>addedNode</var>.
</ol>
</ol>

<h5 id=signaling-slot-change>Signaling slot change</h5>

<p>Each <a>similar-origin window agent</a> has <dfn noexport id=signal-slot-list>signal slots</dfn>
Expand Down Expand Up @@ -2432,14 +2462,8 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
<li><p>Otherwise, <a for=set>insert</a> <var>node</var> into <var>parent</var>'s
<a for=tree>children</a> before <var>child</var>'s <a for=tree>index</a>.

<li><p>If <var>parent</var> is a <a for=Element>shadow host</a> and <var>node</var> is a
<a>slottable</a>, then <a>assign a slot</a> for <var>node</var>.

<li><p>If <var>parent</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a>, and
<var>parent</var> is a <a>slot</a> whose <a for=slot>assigned nodes</a> is the empty list,
then run <a>signal a slot change</a> for <var>parent</var>.

<li><p>Run <a>assign slottables for a tree</a> with <var>node</var>'s <a for=tree>root</a>.
<li><p>If the <i>suppress observers flag</i> is unset, then <a>update slots</a> with « »,
<var>parent</var>, and « <var>node</var> ».
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to run it on both parent and node? We just inserted node into the same tree as parent.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's not what this does, right? It uses parent for different things then node.


<li>
<p>For each <a>shadow-including inclusive descendant</a> <var>inclusiveDescendant</var> of
Expand Down Expand Up @@ -2469,7 +2493,7 @@ before a <var>child</var>, with an optional <i>suppress observers flag</i>, run
</li>
</ol>

<li><p>If <i>suppress observers flag</i> is unset, then <a>queue a tree mutation record</a> for
<li><p>If the <i>suppress observers flag</i> is unset, then <a>queue a tree mutation record</a> for
<var>parent</var> with <var>nodes</var>, « », <var>previousSibling</var>, and <var>child</var>.

<li><p>Run the <a>children changed steps</a> for <var>parent</var>.
Expand Down Expand Up @@ -2559,6 +2583,9 @@ within a <var>parent</var>, run these steps:
<li><p><a for=/>Insert</a> <var>node</var> into <var>parent</var> before <var>referenceChild</var>
with the <i>suppress observers flag</i> set.

<li><p><a>Update slots</a> with « <var>removedNodes</var> », <var>parent</var>, and
« <var>nodes</var> ».

<li><p><a>Queue a tree mutation record</a> for <var>parent</var> with <var>nodes</var>,
<var>removedNodes</var>, <var>previousSibling</var>, and <var>referenceChild</var>.

Expand All @@ -2585,6 +2612,9 @@ within a <var>parent</var>, run these steps:
<li><p>If <var>node</var> is non-null, then <a for=/>insert</a> <var>node</var> into
<var>parent</var> before null with the <i>suppress observers flag</i> set.

<li><p>If the <i>suppress observers flag</i> is unset, then <a>update slots</a> with
« <var>removedNodes</var> », <var>parent</var>, and « <var>addedNodes</var> ».
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the "replace all" algorithm that doesn't have a suppress observers flag at the moment so that would have to be added.


<li><p>If either <var>addedNodes</var> or <var>removedNodes</var> <a for=set>is not empty</a>,
then <a>queue a tree mutation record</a> for <var>parent</var> with <var>addedNodes</var>,
<var>removedNodes</var>, null, and null.
Expand Down Expand Up @@ -2649,21 +2679,8 @@ indicated in the <a for=/>remove</a> algorithm below.

<li><p><a for=set>Remove</a> <var>node</var> from its <var>parent</var>'s <a for=tree>children</a>.

<li><p>If <var>node</var> is <a for=slottable>assigned</a>, then run <a>assign slottables</a> for
<var>node</var>'s <a>assigned slot</a>.

<li><p>If <var>parent</var>'s <a for=tree>root</a> is a <a for=/>shadow root</a>, and
<var>parent</var> is a <a>slot</a> whose <a for=slot>assigned nodes</a> is the empty list,
then run <a>signal a slot change</a> for <var>parent</var>.

<li>
<p>If <var>node</var> has an <a>inclusive descendant</a> that is a <a>slot</a>, then:

<ol>
<li><p>Run <a>assign slottables for a tree</a> with <var>parent</var>'s <a for=tree>root</a>.

<li><p>Run <a>assign slottables for a tree</a> with <var>node</var>.
</ol>
<li><p>If the <i>suppress observers flag</i> is unset, then <a>update slots</a> with
« <var>node</var> », <var>parent</var>, and « ».

<li><p>Run the <a>removing steps</a> with <var>node</var> and <var>parent</var>.

Expand Down Expand Up @@ -2701,7 +2718,7 @@ indicated in the <a for=/>remove</a> algorithm below.
<a for="transient registered observer">source</a> is <var>registered</var> to <var>node</var>'s
<a>registered observer list</a>.

<li><p>If <i>suppress observers flag</i> is unset, then <a>queue a tree mutation record</a> for
<li><p>If the <i>suppress observers flag</i> is unset, then <a>queue a tree mutation record</a> for
<var>parent</var> with « », « <var>node</var> », <var>oldPreviousSibling</var>, and
<var>oldNextSibling</var>.

Expand Down