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.

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

Consider adding localNameFilter to MutationObserver #398

Open
smaug---- opened this issue Jan 15, 2017 · 23 comments
Open

Consider adding localNameFilter to MutationObserver #398

smaug---- opened this issue Jan 15, 2017 · 23 comments
Labels
addition/proposal New features or enhancements needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan

Comments

@smaug----
Copy link
Collaborator

smaug---- commented Jan 15, 2017

I was looking at a slow (cross) browser addon code, and one thing they need to do is to figure out when new input elements have been added to the tree.
This means that all the added (and removed) elements are touched, which forces browsers to have
JS wrappers for those and create MutationRecords too.

localNamefilter, similar to attributeFilter would optimize some of that out.
Using css for filtering might be also one option, but it is way more heavy weight in general, so if possible, I'd stick with localNames. (and localNameFilter could be used for text node filtering too, unlike css)

And while addons aren't part of the web, I think web pages have similar use cases.

@annevk
Copy link
Member

annevk commented Jan 15, 2017

Using selectors is #77. How would localNameFilter be used for text nodes?

@dtapuska @rniwa thoughts?

@rniwa
Copy link
Collaborator

rniwa commented Jan 16, 2017

Filtering based on element names sense to me but why not just elementFilter?

Also, should this follow the usual case-insensitive in HTML but case-sensitive in XML semantics?

@smaug----
Copy link
Collaborator Author

How would localNameFilter be used for text nodes?

Er, silly me, that would need to be nodeName, not localName. And since attributeFilter uses localName, elementFilter should too.

I don't care too much about the name. elementFilter would be ok to me. But it should be case sensitive, similar to attributeFilter.

@annevk annevk added the good first issue Ideal for someone new to a WHATWG standard or software project label Jan 16, 2017
@annevk
Copy link
Member

annevk commented Jan 16, 2017

Case-sensitive elementFilter matching on local name sounds fairly straightforward. I could work on specifying that and writing tests, but also happy to hand it to someone else if there's interest since this seems like a nice starter bug.

@hmntptwl
Copy link

hmntptwl commented Feb 25, 2017

I am ready to check for this chance.Please authorize me.

@annevk
Copy link
Member

annevk commented Feb 27, 2017

@hemant1612 what you want to do is fork this repository and then create a pull request from the fork. The other thing that will need to be done is writing new web-platform-tests.

@zbraniecki
Copy link

Filtering based on element names sense to me but why not just elementFilter?

My main issue with elementFilter is that it solves a subclass of the problem and motivates future additions like attributeFilter or combinations for cases where elements and attributes of particular type are expected.

querySelector handle all of that and can be internally optimized by engines for common cases (like elementFilter, attributeFilter etc.) while keeping the "slow case" still faster than doing that in the JS code

@rniwa
Copy link
Collaborator

rniwa commented Mar 1, 2018

We definitely don't want to execute CSS selectors on every DOM mutation. That's simply non-starter.

@annevk annevk removed the good first issue Ideal for someone new to a WHATWG standard or software project label Mar 1, 2018
@zbraniecki
Copy link

zbraniecki commented Mar 13, 2018

That's simply non-starter.

  1. Why?

For use cases where someone will put it, it basically means that if they can't get it on the MutationObserver config to be performed (and heavily optimized) by the engine, they'll put it in the callback anyway, because their observer clearly needs it.

If the concern is performance, I fail to understand how is it a win.

  1. But, assuming that I'm missing something

Is a combination of localNameFilter and attributeFilter realistic?

@rniwa
Copy link
Collaborator

rniwa commented Mar 13, 2018

That's simply non-starter.

  1. Why?

It would be way too slow.

For use cases where someone will put it, it basically means that if they can't get it on the MutationObserver config to be performed (and heavily optimized) by the engine, they'll put it in the callback anyway, because their observer clearly needs it.

That's okay. Evaluating selectors after the fact is a lot more efficient especially because you'd be executing a lot of them all at once. What's extremely slow is for each DOM mutation to synchronously execute a selector to determine whether a given element should be queued to an observer or not. That's not the kind of work you don't want to be doing for every node being added or removed.

  1. But, assuming that I'm missing something
    Is a combination of localNameFilter and attributeFilter realistic?

That's a lot more realistic filtering assuming attribute filter is about filtering based on the presence of an attribute and not its value.

@smaug----
Copy link
Collaborator Author

But it does change the meaning of attributeFilter, if that starts to affect to MutationRecords created because of childList: true.

@rniwa
Copy link
Collaborator

rniwa commented Mar 14, 2018

@smaug---- Oh yeah, I think we're talking about something different here. It's filtering of records based on attributes, not attributeFilter as it exists today.

@annevk
Copy link
Member

annevk commented Mar 14, 2018

So hasAttributeFilter, effectively? And localNameFilter?

@rniwa
Copy link
Collaborator

rniwa commented Mar 15, 2018

Maybe elementLocalNameFilter and elementAttributesFilter to match the existing naming convention?

@annevk
Copy link
Member

annevk commented Mar 15, 2018

elementFilter (counterpart to attributeFilter) and elementAttributeFilter (singular for attribute to match existing style)? I kinda like elementHasAttributeFilter for the latter as well to make it clearer how it's different.

@annevk annevk added addition/proposal New features or enhancements needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan labels Mar 21, 2020
@annevk
Copy link
Member

annevk commented Mar 21, 2020

It seems there's implementer interest, so what's needed to complete this is people willing to update the DOM Standard and write corresponding tests (ideally different people, but okay if they're the same).

@AndrewRot
Copy link

AndrewRot commented Mar 23, 2020

Closes #850

In which I proposed adding an elementLocalNameFilter as there is no current functionality to support mutation to the DOM involving changes to specific element tags.

With late page hydration seen in modern web frameworks, monitoring changes to elements on the page can be quite useful.


Description for new optional property:

New property: elementLocalNameFilter (optional)

Description: Set to a list of element tag/qualified names if not all element changes need to be observed and subTree is true or omitted.

let observer = new MutationObserver(mutations) => {
  mutations.forEach(mutation => {
    // immediate handle on all new 'input' elements on the page...
  });
});

observer.observe(document.documentElement, {
  childList: true,
  subTree: true,
  elementLocalNameFilter: ['input']
});

Addition to observe method invocations.
The observe(target, options) method, when invoked, must run these steps:
X. If options elementLocalNameFilter is present and options’s subTree is omitted, then set options’s elementChanges to true.
X. If options’s elementLocalNameFilter is present and options’s subTree is false, then throw a TypeError.
(fixing)

@rniwa
Copy link
Collaborator

rniwa commented Mar 25, 2020

For easy of use and consistency with attributeFilter which doesn't callout that it's a local name, I'd suggest elementFilter as the name of the filter.

We can revisit adding a new variant of filtering which takes qualified name (e.g. namespace and/or prefix into account) in the future.

If options elementLocalNameFilter is present and options’s subTree is omitted, then set options’s elementChanges to true.

I don't think there is such a thing as "elementChanges".

If options’s elementLocalNameFilter is present and options’s subTree is false, then throw a TypeError.

I don't think we should do this. It's definitely useful to be able to observe the direct child of an element of specific types.

We'd now have to consider what happens when both attributeFilter and elementFilter are set. One interpretation is that they're independent. We'd be monitoring insertion and removal of elements of specific types, and any changes to a set of attributes. Alternatively, they combine. We'd be only observing changes to attributes on elements of specific types.

@AndrewRot
Copy link

For easy of use and consistency with attributeFilter which doesn't callout that it's a local name, I'd suggest elementFilter as the name of the filter.

I am open to either name for this.

We'd now have to consider what happens when both attributeFilter and elementFilter are set. One interpretation is that they're independent. We'd be monitoring insertion and removal of elements of specific types, and any changes to a set of attributes. Alternatively, they combine. We'd be only observing changes to attributes on elements of specific types.

I have drafted a PR that implements an intersection of elementHasAttributeFilter & elementLocalNameFilter when both are present. This applies the filter to the addedNodes and removedNodes field.

Depending on which implementation strategy is chosen for integrating these features with attributeFilter, I believe it would be best to run each filter, one by one, on the nodes observed in the mutation event, asserting that either addedNodes or removedNodes list is not empty between after each filter that is applied.

Regardless of what is favored for mixing filters (union or intersection), I think it should be consistent for all combinations of filters.

Personally, I believe intersection is easier to use.

Another option is to have an additional operand option which allows the implementor to chose how to combined filters themselves.

@rniwa
Copy link
Collaborator

rniwa commented Apr 3, 2020

Depending on which implementation strategy is chosen for integrating these features with attributeFilter, I believe it would be best to run each filter, one by one, on the nodes observed in the mutation event, asserting that either addedNodes or removedNodes list is not empty between after each filter that is applied.

Yeah, we just need to decide what to do here in order to proceed with this proposal.

Regardless of what is favored for mixing filters (union or intersection), I think it should be consistent for all combinations of filters.

Personally, I believe intersection is easier to use.

I tend to agree although it's unclear what happens to CihldNode observation when an attribute filter is specified. I guess we'd ignore it for child changes?

@rniwa
Copy link
Collaborator

rniwa commented Mar 5, 2021

Filed https://bugs.webkit.org/show_bug.cgi?id=222784 for WebKit.

@def00111
Copy link

I'd suggest elementFilter as the name of the filter.

Why not nodeFilter? This would also match with addedNodes and removedNodes.

@annevk
Copy link
Member

annevk commented Aug 13, 2024

Because it would take element local names and there is precedent with attributeFilter. addedNodes and removedNodes can return non-element nodes and as such cannot have a more specific name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan
Development

No branches or pull requests

7 participants