Skip to content

Commit 7137274

Browse files
annevkannevk
authored
Handle NodeIterator node removal during filtering
The traverse algorithm read the reference into local variables, ran the filter (which can execute author code), and then wrote those locals back. Filtering can remove nodes and run the NodeIterator pre-remove steps, but those adjusted only the stored reference, not the in-progress traversal position held in traverse's locals. As a result a node removed during its own filtering could leave the reference pointing at a detached node, and traversal could continue from there. Model a reference as a node pointer (a tuple of a node and a pointer before), and give each NodeIterator a candidate node pointer that holds the in-progress traversal position. Traverse advances the candidate, sets the reference to it only once a node is accepted, and returns the node passed to the filter. The pre-remove steps adjust both the reference and, while traversing, the candidate. referenceNode continues to report the last accepted node during filtering. Matches Gecko, Chromium, and WebKit. Tests: web-platform-tests/wpt#61035.
1 parent afa6b13 commit 7137274

1 file changed

Lines changed: 97 additions & 41 deletions

File tree

‎dom.bs‎

Lines changed: 97 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2366,7 +2366,7 @@ can be used to explore this matter in more detail.
23662366

23672367
<p>Objects that implement {{Text}} sometimes implement {{CDATASection}}.
23682368

2369-
<p>Thus, every [=node=]'s [=primary interface=] is one of: {{Document}}, {{DocumentType}},
2369+
<p>Thus, every [=/node=]'s [=primary interface=] is one of: {{Document}}, {{DocumentType}},
23702370
{{DocumentFragment}}, {{ShadowRoot}}, {{Element}} or an inherited interface of {{Element}},
23712371
{{Attr}}, {{Text}}, {{CDATASection}}, {{ProcessingInstruction}}, or {{Comment}}.
23722372
</div>
@@ -2970,7 +2970,7 @@ optional boolean <dfn for="insert"><var>suppressObservers</var></dfn> (default f
29702970
<a>post-connection steps</a> <em>while</em> we're traversing the <a>node tree</a>. This is because
29712971
the <a>post-connection steps</a> can modify the tree's structure, making live traversal unsafe,
29722972
possibly leading to the <a>post-connection steps</a> being called multiple times on the same
2973-
<a>node</a>.
2973+
<a for=/>node</a>.
29742974
</li>
29752975

29762976
<li>
@@ -6303,10 +6303,9 @@ method steps are:
63036303
<ol>
63046304
<li><p>Let <var>iterator</var> be a new {{NodeIterator}} object.
63056305

6306-
<li><p>Set <var>iterator</var>'s <a for=traversal>root</a> and <var>iterator</var>'s
6307-
<a for=NodeIterator>reference</a> to <var>root</var>.
6306+
<li><p>Set <var>iterator</var>'s <a for=traversal>root</a> to <var>root</var>.
63086307

6309-
<li><p>Set <var>iterator</var>'s <a for=NodeIterator>pointer before reference</a> to true.
6308+
<li><p>Set <var>iterator</var>'s <a for=NodeIterator>reference</a> to (<var>root</var>, true).
63106309

63116310
<li><p>Set <var>iterator</var>'s <a for=traversal>whatToShow</a> to <var>whatToShow</var>.
63126311

@@ -10312,8 +10311,13 @@ interface NodeIterator {
1031210311
<a for=/>collection</a> rooted at the {{NodeIterator}} object's <a for=traversal>root</a>, whose
1031310312
filter matches any <a for=/>node</a>.
1031410313

10314+
<p>A <dfn>node pointer</dfn> is a <a for=/>tuple</a> consisting of a
10315+
<dfn for="node pointer">node</dfn> (a <a for=/>node</a>) and a
10316+
<dfn for="node pointer">pointer before</dfn> (a boolean).
10317+
1031510318
<p>Each {{NodeIterator}} object also has an associated <dfn for=NodeIterator>reference</dfn> (a
10316-
<a for=/>node</a>) and <dfn for=NodeIterator>pointer before reference</dfn> (a boolean).
10319+
<a>node pointer</a>) and <dfn for=NodeIterator>candidate reference</dfn> (null or a
10320+
<a>node pointer</a>, and initially null).
1031710321

1031810322
<p class=note>As mentioned earlier, {{NodeIterator}} objects have an associated
1031910323
<a for=traversal>is active</a>, <a for=traversal>root</a>, <a for=traversal>whatToShow</a>, and
@@ -10324,31 +10328,49 @@ filter matches any <a for=/>node</a>.
1032410328
given a {{NodeIterator}} object <var>nodeIterator</var> and <a for=/>node</a>
1032510329
<var>toBeRemovedNode</var>, are:
1032610330

10331+
<ol>
10332+
<li><p>Set <var>nodeIterator</var>'s <a for=NodeIterator>reference</a> to the result of
10333+
<a for=NodeIterator>adjusting a node pointer</a> given <var>nodeIterator</var>'s
10334+
<a for=NodeIterator>reference</a>, <var>nodeIterator</var>, and <var>toBeRemovedNode</var>.
10335+
10336+
<li><p>If <var>nodeIterator</var>'s <a for=NodeIterator>candidate reference</a> is non-null, then set
10337+
<var>nodeIterator</var>'s <a for=NodeIterator>candidate reference</a> to the result of
10338+
<a for=NodeIterator>adjusting a node pointer</a> given <var>nodeIterator</var>'s
10339+
<a for=NodeIterator>candidate reference</a>, <var>nodeIterator</var>, and <var>toBeRemovedNode</var>.
10340+
</ol>
10341+
</div>
10342+
10343+
<hr>
10344+
10345+
<div algorithm>
10346+
<p>To <dfn for=NodeIterator>adjust a node pointer</dfn> given a <a>node pointer</a>
10347+
<var>nodePointer</var>, a {{NodeIterator}} object <var>nodeIterator</var>, and a <a for=/>node</a>
10348+
<var>toBeRemovedNode</var>, perform the following steps. They return a <a>node pointer</a>.
10349+
1032710350
<ol>
1032810351
<li><p>If <var>toBeRemovedNode</var> is not an <a for=tree>inclusive ancestor</a> of
10329-
<var>nodeIterator</var>'s <a for=NodeIterator>reference</a>, or <var>toBeRemovedNode</var> is an
10352+
<var>nodePointer</var>'s <a for="node pointer">node</a>, or <var>toBeRemovedNode</var> is an
1033010353
<a for=tree>inclusive ancestor</a> of <var>nodeIterator</var>'s <a for=traversal>root</a>, then
10331-
return.
10354+
return <var>nodePointer</var>.
1033210355

1033310356
<li>
10334-
<p>If <var>nodeIterator</var>'s <a for=NodeIterator>pointer before reference</a> is true:
10357+
<p>If <var>nodePointer</var>'s <a for="node pointer">pointer before</a> is true:
1033510358

1033610359
<ol>
1033710360
<li><p>Let <var>next</var> be <var>toBeRemovedNode</var>'s first <a>following</a>
1033810361
<a for=/>node</a> that is an <a>inclusive descendant</a> of <var>nodeIterator</var>'s
1033910362
<a for=traversal>root</a> and is not an <a>inclusive descendant</a> of
1034010363
<var>toBeRemovedNode</var>, if there is such a <a for=/>node</a>; otherwise null.
1034110364

10342-
<li><p>If <var>next</var> is non-null, then set <var>nodeIterator</var>'s
10343-
<a for=NodeIterator>reference</a> to <var>next</var> and return.
10344-
10345-
<li><p>Set <var>nodeIterator</var>'s <a for=NodeIterator>pointer before reference</a> to false.
10365+
<li><p>If <var>next</var> is non-null, then return (<var>next</var>, true).
1034610366
</ol>
1034710367

10348-
<li><p>Set <var>nodeIterator</var>'s <a for=NodeIterator>reference</a> to
10349-
<var>toBeRemovedNode</var>'s <a for=tree>parent</a>, if <var>toBeRemovedNode</var>'s
10350-
<a>previous sibling</a> is null; otherwise to the <a>inclusive descendant</a> of
10351-
<var>toBeRemovedNode</var>'s <a>previous sibling</a> that appears last in <a>tree order</a>.
10368+
<li><p>Let <var>newNode</var> be <var>toBeRemovedNode</var>'s <a for=tree>parent</a>, if
10369+
<var>toBeRemovedNode</var>'s <a>previous sibling</a> is null; otherwise the
10370+
<a>inclusive descendant</a> of <var>toBeRemovedNode</var>'s <a>previous sibling</a> that appears
10371+
last in <a>tree order</a>.
10372+
10373+
<li><p>Return (<var>newNode</var>, false).
1035210374
</ol>
1035310375
</div>
1035410376

@@ -10361,12 +10383,13 @@ given a {{NodeIterator}} object <var>nodeIterator</var> and <a for=/>node</a>
1036110383

1036210384
<div algorithm>
1036310385
<p>The <dfn attribute for=NodeIterator><code>referenceNode</code></dfn> getter steps are to return
10364-
<a>this</a>'s <a for=NodeIterator>reference</a>.
10386+
<a>this</a>'s <a for=NodeIterator>reference</a>'s <a for="node pointer">node</a>.
1036510387
</div>
1036610388

1036710389
<div algorithm>
1036810390
<p>The <dfn attribute for=NodeIterator><code>pointerBeforeReferenceNode</code></dfn> getter steps
10369-
are to return <a>this</a>'s <a for=NodeIterator>pointer before reference</a>.
10391+
are to return <a>this</a>'s <a for=NodeIterator>reference</a>'s
10392+
<a for="node pointer">pointer before</a>.
1037010393
</div>
1037110394

1037210395
<div algorithm>
@@ -10385,51 +10408,84 @@ are to return <a>this</a>'s <a for=NodeIterator>pointer before reference</a>.
1038510408
<var>type</var>:
1038610409

1038710410
<ol>
10388-
<li><p>Let <var>node</var> be <var>iterator</var>'s <a for=NodeIterator>reference</a>.
10411+
<li><p>Set <var>iterator</var>'s <a for=NodeIterator>candidate reference</a> to <var>iterator</var>'s
10412+
<a for=NodeIterator>reference</a>.
1038910413

10390-
<li><p>Let <var>beforeNode</var> be <var>iterator</var>'s
10391-
<a for=NodeIterator>pointer before reference</a>.
10414+
<li><p>Let <var>result</var> be null.
1039210415

1039310416
<li>
1039410417
<p>While true:
1039510418

1039610419
<ol>
1039710420
<li>
10398-
<p>If <var>type</var> is <code>next</code>":
10421+
<p>If <var>type</var> is "<code>next</code>":
1039910422

1040010423
<ol>
10401-
<li><p>If <var>beforeNode</var> is false, then set <var>node</var> to the first
10402-
<a for=/>node</a> <a>following</a> <var>node</var> in <var>iterator</var>'s
10403-
<a for=NodeIterator>iterator collection</a>. If there is no such <a for=/>node</a>, then return
10404-
null.
10424+
<li>
10425+
<p>If <var>iterator</var>'s <a for=NodeIterator>candidate reference</a>'s
10426+
<a for="node pointer">pointer before</a> is false:
10427+
10428+
<ol>
10429+
<li><p>Let <var>following</var> be the first <a for=/>node</a> <a>following</a>
10430+
<var>iterator</var>'s <a for=NodeIterator>candidate reference</a>'s <a for="node pointer">node</a> in
10431+
<var>iterator</var>'s <a for=NodeIterator>iterator collection</a>. If there is no such
10432+
<a for=/>node</a>, then <a for=iteration>break</a>.
1040510433

10406-
<li><p>If <var>beforeNode</var> is true, then set it to false.
10434+
<li><p>Set <var>iterator</var>'s <a for=NodeIterator>candidate reference</a> to
10435+
(<var>following</var>, false).
10436+
</ol>
10437+
10438+
<li><p>Otherwise, set <var>iterator</var>'s <a for=NodeIterator>candidate reference</a> to
10439+
(<var>iterator</var>'s <a for=NodeIterator>candidate reference</a>'s <a for="node pointer">node</a>,
10440+
false).
1040710441
</ol>
1040810442

1040910443
<li>
1041010444
<p>Otherwise:
1041110445

1041210446
<ol>
10413-
<li><p>If <var>beforeNode</var> is true, then set <var>node</var> to the first
10414-
<a for=/>node</a> <a>preceding</a> <var>node</var> in <var>iterator</var>'s
10415-
<a for=NodeIterator>iterator collection</a>. If there is no such <a for=/>node</a>, then return
10416-
null.
10447+
<li>
10448+
<p>If <var>iterator</var>'s <a for=NodeIterator>candidate reference</a>'s
10449+
<a for="node pointer">pointer before</a> is true:
1041710450

10418-
<li><p>If <var>beforeNode</var> is false, then set it to true.
10451+
<ol>
10452+
<li><p>Let <var>preceding</var> be the first <a for=/>node</a> <a>preceding</a>
10453+
<var>iterator</var>'s <a for=NodeIterator>candidate reference</a>'s <a for="node pointer">node</a> in
10454+
<var>iterator</var>'s <a for=NodeIterator>iterator collection</a>. If there is no such
10455+
<a for=/>node</a>, then <a for=iteration>break</a>.
10456+
10457+
<li><p>Set <var>iterator</var>'s <a for=NodeIterator>candidate reference</a> to
10458+
(<var>preceding</var>, true).
10459+
</ol>
10460+
10461+
<li><p>Otherwise, set <var>iterator</var>'s <a for=NodeIterator>candidate reference</a> to
10462+
(<var>iterator</var>'s <a for=NodeIterator>candidate reference</a>'s <a for="node pointer">node</a>,
10463+
true).
1041910464
</ol>
1042010465

10421-
<li><p>Let <var>result</var> be the result of <a for=/>filtering</a> <var>node</var> within
10422-
<var>iterator</var>.
10466+
<li><p>Let <var>node</var> be <var>iterator</var>'s <a for=NodeIterator>candidate reference</a>'s
10467+
<a for="node pointer">node</a>.
1042310468

10424-
<li><p>If <var>result</var> is {{NodeFilter/FILTER_ACCEPT}}, then <a for=iteration>break</a>.
10425-
</ol>
10469+
<li><p>Let <var>filterResult</var> be the result of <a for=/>filtering</a> <var>node</var> within
10470+
<var>iterator</var>. If this throws an exception, then set <var>iterator</var>'s
10471+
<a for=NodeIterator>candidate reference</a> to null and rethrow that exception.
1042610472

10427-
<li><p>Set <var>iterator</var>'s <a for=NodeIterator>reference</a> to <var>node</var>.
10473+
<li>
10474+
<p>If <var>filterResult</var> is {{NodeFilter/FILTER_ACCEPT}}:
1042810475

10429-
<li><p>Set <var>iterator</var>'s <a for=NodeIterator>pointer before reference</a> to
10430-
<var>beforeNode</var>.
10476+
<ol>
10477+
<li><p>Set <var>iterator</var>'s <a for=NodeIterator>reference</a> to <var>iterator</var>'s
10478+
<a for=NodeIterator>candidate reference</a>.
1043110479

10432-
<li><p>Return <var>node</var>.
10480+
<li><p>Set <var>result</var> to <var>node</var>.
10481+
10482+
<li><p><a for=iteration>Break</a>.
10483+
</ol>
10484+
</ol>
10485+
10486+
<li><p>Set <var>iterator</var>'s <a for=NodeIterator>candidate reference</a> to null.
10487+
10488+
<li><p>Return <var>result</var>.
1043310489
</ol>
1043410490
</div>
1043510491

0 commit comments

Comments
 (0)