While working on adding anonymous content inspection in Bug 777674, I'm realizing that there is an issue with detecting changes to pseudo elements. The MutationObserver does not include additions/removals to these elements. Basically, if a style rule is removed that causes the pseudo element to no longer be an anonymous child of the parent, I'd like to know. Likewise if a new style rule is added. I've put together a basic test case here: http://jsfiddle.net/bgrins/3tAAW/.

I've been working on a workaround in JS by listening to "StyleSheetRemoved" / "StyleSheetAdded" / "StyleRuleRemoved" / "StyleRuleAdded", StyleRuleChanged", detecting if the changed selector contains ":before" or ":after", then triggering a fake "childList" mutation on the element. It sort of works, but I'm running into some issues when these events are sent quickly. I could spend more time trying to make this more reliable (and to try to speed it up), but I'm wondering if there is a possibility to be notified of relevant changes from the platform, either via a MutationObserver or some other event.

I suspect doing this via MutationObserver would be an obervable spec violation. Doing some other event might be possible. However it might fire a good bit more than you might expect, since we will blow away and recreate ::before/::after in all sorts of cases when the box tree changes...

At least this shouldn't end up to the MutationObserver spec. ::before and ::after aren't in DOM tree. We could perhaps add some chrome only way to observe all the changes, though it is not quite clear what kind of MutationRecord should be created for changes in native anonymous content. The parent node doesn't have any child nodes added or removed.

(In reply to Olli Pettay [:smaug] from comment #3) > At least this shouldn't end up to the MutationObserver spec. ::before and > ::after aren't in > DOM tree. > > We could perhaps add some chrome only way to observe all the changes, though > it is not > quite clear what kind of MutationRecord should be created for changes in > native anonymous content. > The parent node doesn't have any child nodes added or removed. OK, makes sense. What would it take to add a new chrome only way to observe changes to native anonymous content? If I was notified of the change along with the parent node this should be enough information. (In reply to Boris Zbarsky [:bz] from comment #2) > Doing some other event might be possible. However it might fire a good bit > more than you might expect, since we will blow away and recreate > ::before/::after in all sorts of cases when the box tree changes... I think it would be OK even if it fired often. We queue up the mutations on the devtools server, so any duplicates could be removed before sending them onto the frontend (so it wouldn't be too noisy). What would be important to make this work would be that if there was still at least one applicable rule it would not fire: element::before {content:'0';} element::before {content:'1';} <--- removing just this one would not trigger a change, since there is still a ::before

So if I have a ::before styled by two rules, and I remove one of them, you're OK with a remove event followed by an add event?

(In reply to Boris Zbarsky [:bz] from comment #5) > So if I have a ::before styled by two rules, and I remove one of them, > you're OK with a remove event followed by an add event? Yes, I believe that would be OK. Though I would like to clarify a couple of things: 1) During the 'remove' event when one of two rules was removed, if I were to query the firstChild() of the parent using the deepTreeWalker with anonymous content inspection on, would it be the _moz_generated_content_before? 2) During the 'remove' event if there was only one rule and it was removed, would firstChild() be the _moz_generated_content_before? If the answer to 1 was yes and 2 was no, then it would be quite easy to only send the events to the frontend at the correct time. However, as long as there was any kind of notification of changes I could always send a new childList request to the frontend, which may cause extra refreshes and a UI challenge but would be workable. The challenge I'm talking about is if I modified the first rule's selector in the rule view and a full child listing happened, then the pseudo element would become deselected and its parent would be selected. I believe there would be a way to work around this, similar to how we deal with outerHTML editing (in which a new childList event happens with the currently selected node in the set). Plus this would not be a common occurrence at all.

> 1) During the 'remove' event when one of two rules was removed, if I were to query the > firstChild() of the parent using the deepTreeWalker with anonymous content inspection > on, would it be the _moz_generated_content_before? Yes. The event would fire async, after we had already created the new ::before. > 2) During the 'remove' event if there was only one rule and it was removed, would > firstChild() be the _moz_generated_content_before? No. You'd probably need these things triggered not just on rule removals but also on any DOM mutation or style change that reframes any ancestor of anything with ::before/::after, I assume (or that creates a ::before/::after).

(In reply to Boris Zbarsky [:bz] from comment #7) > > 1) During the 'remove' event when one of two rules was removed, if I were to query the > > firstChild() of the parent using the deepTreeWalker with anonymous content inspection > > on, would it be the _moz_generated_content_before? > > Yes. The event would fire async, after we had already created the new > ::before. > > > 2) During the 'remove' event if there was only one rule and it was removed, would > > firstChild() be the _moz_generated_content_before? > > No. > OK, so I believe that when a 'remove' event comes through to the devtools server, I can just check and see if there is a moz_generated_content_before on the element and ignore it if so. Adds are a bit trickier, because I would not be sure of whether it was there before or not, but I think it shouldn't be a problem to work around this on the frontend (which would actually know about it, as it would never receive the initial removal). > You'd probably need these things triggered not just on rule removals but > also on any DOM mutation or style change that reframes any ancestor of > anything with ::before/::after, I assume (or that creates a > ::before/::after). Good point - absolutely. This would be especially hard to simulate or track from JS.

One other note: At some point we'll start supporting multiple ::before/::after per element (either when we add that bit of the generated content spec or when we start doing display:contents or whatever it's called now, though I'm not sure how devtools would want to show that situation). That will likely complicate things somewhat...

(In reply to Boris Zbarsky [:bz] from comment #2) > I suspect doing this via MutationObserver would be an obervable spec > violation. Can't we add another type of events which you could opt in to, potentially only for chrome consumers, like "anonChildList" or something? Is that a dumb idea?

(In reply to :Gijs Kruitbosch from comment #10) > (In reply to Boris Zbarsky [:bz] from comment #2) > > I suspect doing this via MutationObserver would be an obervable spec > > violation. > > Can't we add another type of events which you could opt in to, potentially > only for chrome consumers, like "anonChildList" or something? Is that a dumb > idea? (err, to be clear, my point being that I'm not sure that's an observable spec violation, apart from the "if you feed it stuff that isn't defined by the spec, you get back stuff that's not defined by the spec" - I didn't think there was a specced way to query the MutationObserverInit currently in use even if you had a reference to both the observer and the node, per http://dom.spec.whatwg.org/#mutationobserverinit )

> if you feed it stuff that isn't defined by the spec, you get back stuff that's not > defined by the spec The spec should be defining behavior for all inputs. If it's not, it's not a reasonable web spec.

(In reply to :Gijs Kruitbosch from comment #10) > (In reply to Boris Zbarsky [:bz] from comment #2) > > I suspect doing this via MutationObserver would be an obervable spec > > violation. > > Can't we add another type of events which you could opt in to, potentially > only for chrome consumers, like "anonChildList" or something? Is that a dumb > idea? (In reply to Boris Zbarsky [:bz] from comment #12) > > if you feed it stuff that isn't defined by the spec, you get back stuff that's not > > defined by the spec > > The spec should be defining behavior for all inputs. If it's not, it's not > a reasonable web spec. It would be extremely convenient to be able to use the existing MutationObserver machinery for the devtools server. But if it is not reasonable to tack on chrome-only unspecced behavior, we should be able to make something work using a different interface. What would you suggest? Maybe something like styleSheetChangeEventsEnabled? doc.pseudoElementEventsEnabled = true; tabActor.browser.addEventListener("PseudoElementAdded", (e) => { e.target // <-- parent element for pseudo element which now contains e.pseudoElement as an anonymous child. e.pseudoElement // <-- pseudo element that has been added. }); tabActor.browser.addEventListener("PseudoElementRemoved", (e) => { e.target // <-- parent element for pseudo element which no longer contains e.pseudoElement as an anonymous child. e.pseudoElement // <-- pseudo element that has been removed. });

Tacking on chrome-only behavior is ok, as long as content can't observe it.

(In reply to Boris Zbarsky [:bz] from comment #14) > Tacking on chrome-only behavior is ok, as long as content can't observe it. Pardon my ignorance, but I'm not sure how this functionality be added without allowing content to observe it. Could there be a new chrome-only function declared, like MutationObserver.observePseudoElements(node) that fires the same callback as the regular observe function? This would fire the callback with mutation.type as a new string that could be handled by the callback function.

> I'm not sure how this functionality be added without allowing content to observe it That's probably a question for Olli. See comment 3.

We could allow certain MutationObserverInit properties from chrome caller only. But it is still not clear to me what kind of mutationrecord we'd create. childList wouldn't be the right type, but something else. Perhaps anonymousChild and then added/removedNodes would contain the relevant child. How would this all work in case of shadow DOM? Or does it need to?

So before adding any chrome-only API to MutationObserver, would be nice to know the exact requirements, including native anonymous content (internals of form controls etc, and ::before/::after), XBL and Shadow DOM.

(In reply to Olli Pettay [:smaug] from comment #18) > So before adding any chrome-only API to MutationObserver, would be nice to > know the exact > requirements, including native anonymous content (internals of form controls > etc, and ::before/::after), XBL and Shadow DOM. OK, I will try to go through each one by one: Native anonymous content (::before/::after): We need to know when this is added or removed, either as a result of a style rule modification but also when the DOM changes such that a pseudo element is added or removed. Native anonymous content (internals of form controls): I'm not 100% sure here. These are more complicated than ::before/::after because there could be more changes than just added/removed - for example, attributes could change on these elements. After looking at this I'm not sure if we are actually going to want to show this content in the markup view, at least by default - it's quite confusing to see a <scrollbar> element underneath a <select multiple>, for instance. I'm not sure how easy it will be to distinguish this from other anonymous content (XBL/::before), but I'm inclined to not show this content at all in the markup view. XBL Anonymous content: These actually seems to already fire. If I open chrome://browser/content/devtools/scratchpad.xul with the patches from Bug 777674 applied, then modify an attribute on one of the <xul:label> elements under one of, then I receive an "attributes" mutation. So I don't think there needs to be any chancges here. Shadow DOM: Just like XBL Anonymous content, this actually appears to already fire with the patches applied (I'm not sure if it *should* be, but it seems to be). We haven't fully defined how we will represent this in the markup view (see Comment 53-60 in Bug 777674). It sounds like this is leaning towards showing only the youngest shadow tree only because it is what appears in the composed tree. In addition, we may show an older shadow root alongside a <shadow> insertion point if necessary. We are hoping that tree will appear just like normal elements, so we will want all possible mutations to fire on the trees.

An update to comment 19: I am *not* showing any native anonymous content except for ::before/::after in the markup view. Given that, plus the rest of comment 19, the only requirement for such an API would be to detect mutations on ::before and ::after elements. I haven't worked on these APIs at all - do you think I could tackle this with mentorship or would it require more experience?

I'm working on an extension that needs to observe changes to native anonymous content of form elements, so it'd be great if whatever we come up with here also supported doing that, even if there's no immediate use for it in Firefox code.

(In reply to Brian Grinstead [:bgrins] from comment #20) > I haven't worked on these APIs at all - do you think I could tackle this > with mentorship or would it require more experience? This is a bit tricky one to implement cleanly and in a way which doesn't slow down stuff too much. (it is still not quite clear to me how to add this stuff to MutationObserver API)

I guess BindToTree implementation could call something like nsNodeUtils::NativeAnonymousNodeBound and that would call relevant observers (if we know there are observers wanting that information).

Would love to get this in for 40. I don't have a good idea how much work it's going to be, though.

Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug

(In reply to Brian Grinstead [:bgrins] from comment #26) > Created attachment 8661043 [details] > MozReview Request: Bug 1034110 - Provide a way to observe mutations for > ::before/::after pseudo elements;r=smaug > > Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo > elements;r=smaug I don't think this is actually ready for review, just looking for some feedback. I tried to copy how other MutationObserverInit options were implemented, and it actually seems to work in my test pages. I'll keep working on the test in the mean time.

I'm not quite happy with naming. NativeAnonymousNodeChanged hints about any kind of change, but what it actually is root-native-anonymous-node-bound-to-observed-node or root-native-anonymous-node-unbound-from-observed-node. Right now I don't really have any good suggestion what the name should be... perhaps just nativeAnonymousChildList in the MutationObserver API and NativeAnonymousChildListChange in the nsIMutationObserver API Please use mozilla coding style consistently. So, aFoo for arguments and such ;) Do devtools access native anonymous elements regularly?

(In reply to Olli Pettay [:smaug] from comment #28) > Do devtools access native anonymous elements regularly? We have a wrapper [0] for an inIDeepTreeWalker. In the normal case (web toolbox) we skip all anonymous content except for ::before/::after (unless if it's a XUL doc, then we include it) [1]. In the Browser Toolbox we show all native anonymous content [2]. I'm most interested about notifications for ::before and ::after, although notifications about any native anonymous content should be OK since we are immediately filtering out nodes that the Walker doesn't know about (and not sending them over the protocol) [3]. [0]: https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/toolkit/devtools/server/actors/inspector.js#3764 [1]: https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/toolkit/devtools/server/actors/inspector.js#3859 [2]: https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/toolkit/devtools/server/actors/inspector.js#3890 [3]: https://dxr.mozilla.org/mozilla-central/rev/9ed17db42e3e46f1c712e4dffd62d54e915e0fac/toolkit/devtools/server/actors/inspector.js#2777

(In reply to Brian Grinstead [:bgrins] from comment #29) > In the Browser Toolbox we show > all native anonymous content [2]. Even the contents of form elements like <input>, or UI element of <audio>/<video> ? But anyhow, that is up to devtools to deal with.

Comment on attachment 8661043 [details] MozReview Request: Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug

(In reply to Olli Pettay [:smaug] from comment #30) > (In reply to Brian Grinstead [:bgrins] from comment #29) > > In the Browser Toolbox we show > > all native anonymous content [2]. > Even the contents of form elements like <input>, or UI element of > <audio>/<video> ? > > But anyhow, that is up to devtools to deal with. Yes, the Browser Toolbox shows all that stuff.

(In reply to Olli Pettay [:smaug] from comment #28) > I'm not quite happy with naming. NativeAnonymousNodeChanged hints about any > kind of change, but what it actually is > root-native-anonymous-node-bound-to-observed-node or > root-native-anonymous-node-unbound-from-observed-node. > Right now I don't really have any good suggestion what the name should be... > perhaps just nativeAnonymousChildList in the MutationObserver API and > NativeAnonymousChildListChange in the nsIMutationObserver API > > Please use mozilla coding style consistently. So, aFoo for arguments and > such ;) > > Do devtools access native anonymous elements regularly? I've updated the naming and the aFoo style. Also have updated the test quite a bit to make sure that the reference to the changed nodes is correct using a deep tree walker. I'm not sure if there is a way to bundle mAddedNodes and mRemovedNodes into a single record when one is being added and one is being removed (or if it's even worth doing that). For the devtools use-case, it's not really a big deal one way or another.

Comment on attachment 8661043 [details] MozReview Request: Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug You need to update also nsGenericDOMDataNode::BindToTree/UnbindFromTree And I would move + if (this->IsRootOfNativeAnonymousSubtree()) { + nsNodeUtils::NativeAnonymousChildListChange(this, true); + } to happen right before if (GetParent()) { (GetParent returns something if we have parent which isn't Document) Does devtools deal with the case when there is native anonymous subtree under some other native anonymous subtree? Since that it possible, IIRC, at least with videocontrols. Perhaps we want to observe changes only in the current subtree, not in nested ones. +nsMutationReceiver::NativeAnonymousChildListChange(nsIDocument* aDocument, + mozilla::dom::Element* aElement, + bool aIsRemove) { + if (!NativeAnonymousChildList()) { + return; + } We need to also check something like nsINode* parent = aElement->GetParentNode(); (Subtree() && RegisterTarget()->SubtreeRoot() == parent->SubtreeRoot()) || parent == Target(); and only create the record if that is right + + // XXX: Can there already be a currentrecord for this? Yes, if you have Subtree() and you're observing several ancestors. And if we have a record already, we can just return early.

Comment on attachment 8661043 [details] MozReview Request: Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug Adds a new chrome-only MutationObserverInit option called nativeAnonymousChildList that will cause a mutation to fire when a native anonymous root is bound or unbound

Bug 1034110 - Listen to nativeAnonymousChildList changes from devtools;r=pbrosset

(In reply to Olli Pettay [:smaug] from comment #34) I've pushed a new version that separates the devtools specific bits and addresses the comments. See notes below > Comment on attachment 8661043 [details] > MozReview Request: Bug 1034110 - Provide a way to observe mutations for > ::before/::after pseudo elements;r=smaug > > You need to update also > nsGenericDOMDataNode::BindToTree/UnbindFromTree Done. In the process I had to switch the type to nsIContent instead of Element. I couldn't think of a way to add test coverage for this since I wasn't aware of cases where those generic elements have anonymous children. I can update the test to add that if you know of a case. > And I would move > + if (this->IsRootOfNativeAnonymousSubtree()) { > + nsNodeUtils::NativeAnonymousChildListChange(this, true); > + } > to happen right before if (GetParent()) { > (GetParent returns something if we have parent which isn't Document) > > > Does devtools deal with the case when there is native anonymous subtree > under some other native anonymous subtree? > Since that it possible, IIRC, at least with videocontrols. Perhaps we want > to observe changes only in the current subtree, not in > nested ones. > > +nsMutationReceiver::NativeAnonymousChildListChange(nsIDocument* aDocument, > + mozilla::dom::Element* > aElement, > + bool aIsRemove) { > + if (!NativeAnonymousChildList()) { > + return; > + } > We need to also check something like > nsINode* parent = aElement->GetParentNode(); > (Subtree() && RegisterTarget()->SubtreeRoot() == parent->SubtreeRoot()) || > parent == Target(); > and only create the record if that is right > Done, I think the early returns are now handling both subtree and non-subtree cases properly, but please have a look. I also added test coverage for both of these cases. > > > + > + // XXX: Can there already be a currentrecord for this? > Yes, if you have Subtree() and you're observing several ancestors. > And if we have a record already, we can just return early. I'm not sure that I've addressed this. How do I check if there is a record? Just `if (m->mTarget)`?

Comment on attachment 8661043 [details] MozReview Request: Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug +/** + * Notification that the root of a native anonymous subtree + * (such as ::before or ::after) has been added or removed. + * This fires for chrome observers only. + * + * @param aDocument The owner-document of aContent. + * @param aContent The anonymous node that's been added or removed. + * @param aIsRemove True if it's a removal, false for addition. + */ +void +nsMutationReceiver::NativeAnonymousChildListChange(nsIDocument* aDocument, + nsIContent* aContent, + bool aIsRemove) { That comment above the method is in a bit odd place. And the notification fires for all observers. So, just remove. NativeAnonymousChildListChange should be, and is documented in nsIMutationObserver.h Update IID of nsIMutationObserver In Element::BindToTree nsNodeUtils::NativeAnonymousChildListChange should be called only if !hadParent So, if (!hadParent && IsRootOfNativeAnonymousSubtree()) { nsNodeUtils::NativeAnonymousChildListChange(this, true); } and for nsGenericDOMDataNode::BindToTree you need to add the similar hadParent bool variable (initialized before // Set parent) and then similar if check as in Element. + nsDOMMutationRecord* m = + Observer()->CurrentRecord(nsGkAtoms::nativeAnonymousChildList); and after that if m->mTarget is non-null, you can return. bool attributeOldValue = aOptions.mAttributeOldValue.WasPassed() && - aOptions.mAttributeOldValue.Value(); + aOptions.mAttributeOldValue.Value(); + bool nativeAnonymousChildList = aOptions.mNativeAnonymousChildList && + nsContentUtils::ThreadsafeIsCallerChrome(); Why you un-indent aOptions.mAttributeOldValue.Value()? "*Notification that an anonymous element has been added or * removed." in nsIMutationObserver should say that the notification is about adding or removing a root of a native anonymous tree. (and definitely say _native_, since this notification isn't about XBL or anything, which is non-native anonymous stuff) testDifferetTargetNoSubtree ? Different, right?

Comment on attachment 8664525 [details] MozReview Request: Bug 1034110 - Listen to nativeAnonymousChildList changes from devtools;r=pbrosset https://reviewboard.mozilla.org/r/19981/#review18091 Wow, the devtools part of this change really is super simple. That's great! No comments on these code changes.

Comment on attachment 8661043 [details] MozReview Request: Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug Adds a new chrome-only MutationObserverInit option called nativeAnonymousChildList that will cause a mutation to fire when a native anonymous root is bound or unbound

Comment on attachment 8664525 [details] MozReview Request: Bug 1034110 - Listen to nativeAnonymousChildList changes from devtools;r=pbrosset Bug 1034110 - Listen to nativeAnonymousChildList changes from devtools;r=pbrosset

https://reviewboard.mozilla.org/r/19255/#review18093 ::: dom/base/nsIMutationObserver.h:25 (Diff revisions 3 - 4) > -{ 0xdd74f0cc, 0x2849, 0x4d05, \ > - { 0x9c, 0xe3, 0xb0, 0x95, 0x3e, 0xc2, 0xfd, 0x44 } } > +{ 0x6d674c17, 0x0fbc, 0x4633, \ > + { 0x8f, 0x46, 0x73, 0x4e, 0x87, 0xeb, 0xf0, 0xc7 } } Hm, not sure why reviewboard re-flagged the review. Anyway, I just want to be sure this is all I need to do for updating the IID for these changes?

yes. And MozReview is silly and causes tons of bugmail spam by adding review requests again and again.

Comment on attachment 8661043 [details] MozReview Request: Bug 1034110 - Provide a way to observe mutations for ::before/::after pseudo elements;r=smaug Clearing review flag

https://hg.mozilla.org/integration/fx-team/rev/3a8904e50016 https://hg.mozilla.org/integration/fx-team/rev/7b145ea2fbde

Backed out for bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=4818434&repo=fx-team https://hg.mozilla.org/integration/fx-team/rev/e9aea2b7d9fd

Also want to blame these patches for https://treeherder.mozilla.org/logviewer.html#?job_id=4817750&repo=fx-team

(In reply to Wes Kocher (:KWierso) from comment #46) > Backed out for bustage: > https://treeherder.mozilla.org/logviewer.html#?job_id=4818434&repo=fx-team > https://hg.mozilla.org/integration/fx-team/rev/e9aea2b7d9fd Hm, that passed yesterday on my try pushes but can reproduce it locally. It looks like a timing issue where the setTimeout is firing before the pseudo element is removed. I have a test fix which I've pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=06ee94492294. Will retrigger dt also to see if that markupview failure happens.

Comment on attachment 8664525 [details] MozReview Request: Bug 1034110 - Listen to nativeAnonymousChildList changes from devtools;r=pbrosset Moving devtools-specific work into Bug 1208544