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

MutationObserver does not see changes within a shadow-root #110

Closed
jordanaustin opened this issue Feb 8, 2016 · 9 comments
Closed

MutationObserver does not see changes within a shadow-root #110

jordanaustin opened this issue Feb 8, 2016 · 9 comments

Comments

@jordanaustin
Copy link

Currently the MutationObserver cannot observe changes within a shadow-root unless explicitly set on the fragment. This is a problem for features like maintain.disabled where we observe changes on a context and filtered elements. As of right now any changed made to those shadow-roots will not be observed.

The purposed solution suggested by @rodneyrehm on #107 is to check for a shadow-root on all child elements of the context/filters and hookup an observer on all of the shadow-roots individually.

@jordanaustin
Copy link
Author

I can take this bug on.

@rodneyrehm
Copy link
Member

Since we need this kind of utility in mutliple places (at least 2), it makes sense to create some sort of abstraction (I called these things "service") for "recursive MutationObservers".

@jordanaustin
Copy link
Author

Yeah a service to handle this would be a good idea. I'll look at how you've handled services and work on one for this.

@rodneyrehm
Copy link
Member

First, I don't think we need to observe shadows for maintain/hidden after all.

Registering a MutationObserver for each existing ShadowHost is not a problem upon engaging. Finding ShadowHosts existing upon an observed mutation is not a problem either. But we can't observe a mutation when element.createShadowRoot() is called, as this bin shows. I've opened whatwg/dom#170 to find out if we'll ever be able to do this.

@jordanaustin
Copy link
Author

Good findings, thanks for following up on this.

@rodneyrehm rodneyrehm added this to the 1.1.0 - second wave milestone Feb 22, 2016
@rodneyrehm
Copy link
Member

@jordanaustin are you still planning on giving this a shot, or should I take over? If you're up for it, I'm happy to help with docs and tests… I'd be looking to close this soon, as it's one of two remaining issues for the 1.1.0 release.


I guess I'd have created ally/query/shadows that uses a TreeWalker to find ShadowHosts, much like query/focusable.strict.js does. Considering a TreeWalker takes a context, the same function can be used to scan maintain/disabled's engaging context and scan the added nodes identified by MutationObserver. I would probably try extracting the MutationObserver from maintain/disabled to observe/shadows - a new service that has the a similar surface API to MutationObserver, but also observes any ShadowHosts nested in the engaging context.

@jordanaustin
Copy link
Author

@rodneyrehm sorry I've been slammed and haven't had a chance to look into this recently. If you have time to take it, that won't hurt my feelings. Otherwise I might have a chance to get back to this in a few weeks.

I am keenly interested in seeing a 1.1.0 release however so if this is holding it up I can try to carve some time out over the weekend possibly.

@rodneyrehm
Copy link
Member

No need to apologize… this is OSS, not your dayjob ;) I'll take over :)

@rodneyrehm rodneyrehm self-assigned this Feb 23, 2016
@jordanaustin
Copy link
Author

Thanks @rodneyrehm :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants