Re: [whatwg/dom] Add new mutation observer init options elementFilterByAttribute and elementLocalNameFilter (#885)

@annevk commented on this pull request.

Thank you for confirming the IPR stuff, that's great! There's a couple things remaining with the PR (note that several comments apply to multiple places) and the other thing we'll need here are tests. Do you think you can work on those?

> @@ -3594,6 +3625,67 @@ run these steps:
 <ol>
  <li><p>Assert: either <var>addedNodes</var> or <var>removedNodes</var> <a for=set>is not empty</a>.
 
+ <li>
+  <p>If either <var>options</var>'s' {{MutationObserverInit/elementFilter}} or
+  {{MutationObserverInit/elementByAttributeFilter}} is present, then:
+
+  <ol>
+   <li><p>Let <var>filteredAddedNodes</var> be an empty <a for=/>list</a>.
+
+   <li><p>Let <var>filterRemovedNodes</var> be an empty <a for=/>list</a>.
+
+   <li>
+    <p>For each <var>node</var> in <var>addedNodes</var></a>:
+
+    <ol>
+     <li><p>Let <var>nodeAttributesList</var> be the set of local names of <var>node</var>'s
+     attributes.

"local names" and "attributes" should xref the relevant definitions. (We should also have a test with two attributes that have the same local name and see if this is all working as designed.)

> +   <li><p>Let <var>filteredAddedNodes</var> be an empty <a for=/>list</a>.
+
+   <li><p>Let <var>filterRemovedNodes</var> be an empty <a for=/>list</a>.
+
+   <li>
+    <p>For each <var>node</var> in <var>addedNodes</var></a>:
+
+    <ol>
+     <li><p>Let <var>nodeAttributesList</var> be the set of local names of <var>node</var>'s
+     attributes.
+
+     <li>
+      <p>If none of the following are true
+
+      <ul class=brief>
+       <li>if <var>options</var>'s {{MutationObserverInit/elementFilter}} is present, and the

I don't think the nested "if" works here. This needs rephrasing.

> +
+   <li><p>Let <var>filterRemovedNodes</var> be an empty <a for=/>list</a>.
+
+   <li>
+    <p>For each <var>node</var> in <var>addedNodes</var></a>:
+
+    <ol>
+     <li><p>Let <var>nodeAttributesList</var> be the set of local names of <var>node</var>'s
+     attributes.
+
+     <li>
+      <p>If none of the following are true
+
+      <ul class=brief>
+       <li>if <var>options</var>'s {{MutationObserverInit/elementFilter}} is present, and the
+       <a>node</a>'s <var>localName</var> is in {{MutationObserverInit/elementFilter}}

This is quite confusing. Isn't node the variable and local name the field?

> +     <li><p>Let <var>nodeAttributesList</var> be the set of local names of <var>node</var>'s
+     attributes.
+
+     <li>
+      <p>If none of the following are true
+
+      <ul class=brief>
+       <li>if <var>options</var>'s {{MutationObserverInit/elementFilter}} is present, and the
+       <a>node</a>'s <var>localName</var> is in {{MutationObserverInit/elementFilter}}
+
+       <li><var>options</var>'s {{MutationObserverInit/elementByAttributeFilter}} is present, and
+       <var>options</var>'s {{MutationObserverInit/elementByAttributeFilter}} contains any of the
+       attribute name in <var>nodeAttributesList</var>
+      </ul>
+
+      <p>then <a for=queue>enqueue</a> <var>record</var> node in <var>filteredAddedNodes</var>.

I think you might mean `<a for=list>append</a> <var>node</var> to <var>filteredAddedNodes</var>` here?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/pull/885#pullrequestreview-495473603

Received on Thursday, 24 September 2020 11:38:50 UTC