Add new mutation observer init options elementByAttributeFilter and elementFilter #885

AndrewRot · 2020-08-12T22:30:00Z

Adds additional options when constructing a MutationObserver.

MutationObserverInit now contains two additional options: elementByAttributeFilter and elementFilter which are used to enhance the precision of filtering DOM nodes to observe.

See #398 for the background.

At least two implementers are interested (and none opposed):
- @rniwa
- @annevk ?
Tests are written and can be reviewed and commented upon at:
- … TODO
Implementation bugs are filed:
- Chrome: None yet
- Firefox: None yet
- Safari: None yet

(See WHATWG Working Mode: Changes for more details.)

Preview | Diff

dom.bs

annevk · 2020-08-14T13:45:42Z

@AndrewRot can Shape Security sign https://participate.whatwg.org/agreement? Previously that ended up not happening, but not sure what the reason was.

dom.bs

…elementLocalNameFilter

annevk

Generally this looks great, thanks for working on this! I have left a couple of formatting nits and I'm happy to help with those once we're further along.

We'll need a lot of tests to cover the various conditions and new behavior of observe().

I'm a little concerned about the agreement. Typically a person in a management or a very senior engineer would sign it (or at least be an alternate), but neither appears to be the case here, assuming I have interpreted your title correctly. As it seems this is a very big company, have legal et al signed off on this?

dom.bs

annevk · 2020-09-10T09:17:15Z

dom.bs

+ {{MutationObserverInit/elementFilterByAttribute}} is present, then:
+
+ <ol>
+  <li><p>Let <var>nodeAttributesList</var> be the set of local names of node's attributes.


What is node referring to here? I feel like this is lacking some references to other variables.

annevk · 2020-09-10T09:17:58Z

dom.bs

@@ -3594,6 +3625,60 @@ 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/elementLocalNameFilter}} or
+ {{MutationObserverInit/elementFilterByAttribute}} is present, then:


Since this <p> isn't the only child of the <li>, it needs to be on its own line, indented one. This also goes for the following <ol>, which is not indented accurately.

dom.bs

annevk · 2020-09-10T09:18:54Z

dom.bs

+  <li>
+   <p>For each <var>node</var> in <var>addedNodes</var></a>.
+
+   <li>


I would expect a for each to be followed by a nested list so there's an <ol> missing here I think.

annevk · 2020-09-10T09:19:27Z

dom.bs

+     <p>If none of the following are true
+
+     <ul class=brief>
+      <li>if <var>options</var>'s


s/if// I think? Same below. If not I'd try to reword these to be more clearly true/false statements.

annevk · 2020-09-10T09:23:48Z

dom.bs

+     <ul class=brief>
+      <li>if <var>options</var>'s
+      {{MutationObserverInit/elementLocalNameFilter}} is present, and
+      "<code>node.localName</code>" is in {{MutationObserverInit/elementLocalNameFilter}}


What is node.localName and why is it formatted this way? Is that a special value?

annevk · 2020-09-10T10:15:00Z

@mfreed7 any thoughts from Google on this?

rniwa · 2020-09-11T05:50:27Z

FWIW, we're supportive of this proposal.

AndrewRot · 2020-09-23T19:10:35Z

@annevk yes - I've gotten written approval to sign off on the agreement from our legal department as well as my team managers.

annevk

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?

annevk · 2020-09-24T11:31:53Z

dom.bs

+
+    <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.)

annevk · 2020-09-24T11:32:45Z

dom.bs

+      <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.

annevk · 2020-09-24T11:34:01Z

dom.bs

+
+      <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?

annevk · 2020-09-24T11:36:24Z

dom.bs

+       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?

AndrewRot · 2021-09-22T18:18:08Z

@annevk looks like agreement was filled out. Though I just added my manager as a secondary contact.

All comments appear to have been addressed. At this point, the only thing missing seems to be the tests? I can start on those.

annevk · 2021-09-23T15:38:03Z

@AndrewRot that sounds great, thank you! Once the tests are there I'm happy to give this another pass.

bershanskiy · 2022-12-13T15:58:16Z

Has there been any progress on the tests? I'm working on a product which would benefit from this Mutation Observer filter feature, and might be able to assist with tests if there is interest.

annevk · 2022-12-13T16:23:18Z

@mfreed7 ping on Chromium interest. @smaug---- Gecko?

@bershanskiy I'd be happy to pick up the remaining specification work assuming @AndrewRot no longer has the bandwidth if you can sort the tests. Thanks for volunteering!

AndrewRot · 2022-12-13T16:44:23Z

Hi @bershanskiy & @annevk - I'm glad this feature is useful for others! Yes, feel free to take the reins from here. I do not currently have the bandwidth to allocate for this feature.

smaug---- · 2022-12-13T23:02:10Z

dom.bs

+    </ol>
+
+   <li><p>Assert: either <var>filteredAddedNodes</var> or <var>filteredRemovedNodes</var>
+   <a for=set>is not empty</a>.


How can we assert this?

Both of these are now defined in scope. Check if filteredAddedNodes length > 0 or filteredRemovedNodes length > 0.

smaug---- · 2022-12-13T23:06:38Z

dom.bs

+       attribute names in <var>nodeAttributesList</var>
+      </ul>
+
+      <p>then <a for=list>append</a> <var>node</var> to <var>filterRemovedNodes</var>.


I must be confused here.
Don't we want something like
if elementFilter is present, and elementByAttributeFilter is not present and localName is in elementfilter, or if
elementByAttributeFilter is present and elementFilter is not present and elementByAttributeFilter contains any of the attribute names in nodeAttribute list ... or combination of those two

smaug---- · 2022-12-13T23:09:41Z

Gecko would implement this or some variant of it.
(which isn't super surprising since looks like I filed #398 )

mfreed7 · 2022-12-15T17:29:09Z

@mfreed7 ping on Chromium interest. @smaug---- Gecko?

@bershanskiy I'd be happy to pick up the remaining specification work assuming @AndrewRot no longer has the bandwidth if you can sort the tests. Thanks for volunteering!

Looks like a good addition to MutationObserver which would avoid boilerplate JS and speed things up a lot. I'm supportive!

AndrewRot force-pushed the new-mutation-observer-init-options branch from 624f14f to 020ee4a Compare August 12, 2020 22:33

rniwa reviewed Aug 12, 2020

View reviewed changes

dom.bs Outdated Show resolved Hide resolved

rniwa reviewed Aug 12, 2020

View reviewed changes

dom.bs Outdated Show resolved Hide resolved

rniwa reviewed Aug 12, 2020

View reviewed changes

dom.bs Outdated Show resolved Hide resolved

AndrewRot changed the title ~~Add new mutation observer init options elementHasAttributeNameFilter and elementLocalNameFilter~~ Add new mutation observer init options elementFilterByAttribute and elementLocalNameFilter Aug 13, 2020

AndrewRot force-pushed the new-mutation-observer-init-options branch from e37b25c to 19960d9 Compare August 28, 2020 16:20

This comment has been minimized.

Sign in to view

annevk reviewed Aug 31, 2020

View reviewed changes

dom.bs Outdated Show resolved Hide resolved

Add mutation observer init options elementHasAttributeNameFilter and …

d1c9cb6

…elementLocalNameFilter

AndrewRot force-pushed the new-mutation-observer-init-options branch from d00431a to d1c9cb6 Compare August 31, 2020 16:03

annevk reviewed Sep 10, 2020

View reviewed changes

minor fixes

8dff3a0

fix formatting

e0b4bcb

annevk reviewed Sep 24, 2020

View reviewed changes

minor fixes

cb4b0d9

Base automatically changed from master to main January 15, 2021 07:32

add links

f9d51aa

annevk mentioned this pull request Sep 10, 2021

Editorial: fix a typo in "completely finish loading" note whatwg/html#7048

Merged

AndrewRot changed the title ~~Add new mutation observer init options elementFilterByAttribute and elementLocalNameFilter~~ Add new mutation observer init options elementByAttributeFilter and elementFilter Sep 13, 2021

Merge branch 'main' into new-mutation-observer-init-options

63b316d

smaug---- reviewed Dec 13, 2022

View reviewed changes

annevk mentioned this pull request Sep 27, 2023

MutationOvserver and DOMNodeInsertedIntoDocument #1227

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new mutation observer init options elementByAttributeFilter and elementFilter #885

Add new mutation observer init options elementByAttributeFilter and elementFilter #885

AndrewRot commented Aug 12, 2020 •

edited by pr-preview bot

Loading

annevk commented Aug 14, 2020

This comment has been minimized.

annevk left a comment

annevk Sep 10, 2020

annevk Sep 10, 2020

annevk Sep 10, 2020

annevk Sep 10, 2020

annevk Sep 10, 2020

annevk commented Sep 10, 2020

rniwa commented Sep 11, 2020

AndrewRot commented Sep 23, 2020

annevk left a comment

annevk Sep 24, 2020

annevk Sep 24, 2020

annevk Sep 24, 2020

annevk Sep 24, 2020

AndrewRot commented Sep 22, 2021

annevk commented Sep 23, 2021

bershanskiy commented Dec 13, 2022

annevk commented Dec 13, 2022

AndrewRot commented Dec 13, 2022

smaug---- Dec 13, 2022

AndrewRot Dec 15, 2022

smaug---- Dec 13, 2022

smaug---- commented Dec 13, 2022

mfreed7 commented Dec 15, 2022

Add new mutation observer init options elementByAttributeFilter and elementFilter #885

Are you sure you want to change the base?

Add new mutation observer init options elementByAttributeFilter and elementFilter #885

Conversation

AndrewRot commented Aug 12, 2020 • edited by pr-preview bot Loading

annevk commented Aug 14, 2020

This comment has been minimized.

annevk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Sep 10, 2020

rniwa commented Sep 11, 2020

AndrewRot commented Sep 23, 2020

annevk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndrewRot commented Sep 22, 2021

annevk commented Sep 23, 2021

bershanskiy commented Dec 13, 2022

annevk commented Dec 13, 2022

AndrewRot commented Dec 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smaug---- commented Dec 13, 2022

mfreed7 commented Dec 15, 2022

AndrewRot commented Aug 12, 2020 •

edited by pr-preview bot

Loading