New API to enumerate mutation observers

RESOLVED FIXED in mozilla26

Status

()

Core
DOM: Events
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Honza, Assigned: smaug)

Tracking

({dev-doc-needed})

20 Branch
mozilla26
x86
Windows Vista
dev-doc-needed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
DevTools (e.g Firebug) could use an API to enumerate mutation observers for a node from chrome. 

Ideally, the new API would give the following information for every mutation observer:

1) The mutation observer JS callback function
2) options (MutationObserverInit)

Honza
(Assignee)

Comment 1

4 years ago
So you want a list of MutationObservers which is explicitly observing some node?
What about transient observers? I guess those aren't needed.

I guess we could add

dictionary MutationObservingInfo : MutationObserverInit
{
  Node observedNode;
  MutationCallback callback;
}
[ChromeOnly] readonly attribute sequence<MutationObservingInfo> mutationObservers;
to Node, assuming we don't need anything for C++ addons.

Sounds ok?
(Assignee)

Comment 2

4 years ago
Or perhaps Node should just have
[ChromeOnly] sequence<MutationObserver> getAttachedMutationObservers();

And MutationObserver
[ChromeOnly] sequence<MutationObservingInfo> getObservingInfo();
and [ChromeOnly] readonly attribute MutationCallback callback;
(Reporter)

Comment 3

4 years ago
(In reply to Olli Pettay [:smaug] from comment #2)
> Or perhaps Node should just have
> [ChromeOnly] sequence<MutationObserver> getAttachedMutationObservers();
> 
> And MutationObserver
> [ChromeOnly] sequence<MutationObservingInfo> getObservingInfo();
> and [ChromeOnly] readonly attribute MutationCallback callback;
Yep, I like this.


Honza
(Reporter)

Comment 4

4 years ago
What about dynamic tracking of added/removed observers?

I can imagine a tool that needs to update displayed list of observers when new one is added or existing one removed. Should this be done/discussed in another bug report?

Honza
(Assignee)

Comment 5

4 years ago
added observers would be doable pretty easily (but would need some service probably), 
removed is hard, since observers may die when the node itself die.
(new MutationObserver(function() {})).observe(document.createElement("foo"), {childList: true})
keeps both the created element and mutation observer alive until cycle collector has run.
And we don't want to notify any scriptable service during cycle collection's unlink.
(Assignee)

Comment 6

4 years ago
Or would you need notification only about explicit removal? Hmm, though, then the API would be just inconsistent.
(Reporter)

Comment 7

4 years ago
(In reply to Olli Pettay [:smaug] from comment #6)
> Or would you need notification only about explicit removal?
You are thinking about observer.disconnect(), correct?

> Hmm, though, then the API would be just inconsistent.
Right

I don't have any better idea than only hook explicit removals, but we can also wait till some tools really need it. For now I am thinking about a new Command Line command (in Firebug): getMutationObservers(node)

...similar to what is already for event listeners: getEventListeners()
http://www.softwareishard.com/blog/planet-mozilla/firebug-tip-geteventlisteners-command/

Honza
(Assignee)

Comment 8

4 years ago
Patch coming
Assignee: nobody → bugs
(Assignee)

Comment 9

4 years ago
Created attachment 800273 [details] [diff] [review]
patch + ad hoc tests

Removes IID for nsMutationReceiver since it isn't needed, but adds one for
nsDOMMutationObserver.

I think sicking is on vacation + work week, so perhaps you peterv could review this. Should be rather simple.

https://tbpl.mozilla.org/?tree=Try&rev=d8ca022e34aa
Attachment #800273 - Flags: review?(peterv)
(Reporter)

Comment 10

4 years ago
Wow, that was quick!

Any chance to get a try build?
(Win, 32 bit)

Honza
(Assignee)

Comment 11

4 years ago
https://tbpl.mozilla.org/?tree=Try&rev=d8ca022e34aa gives only debug builds
(Assignee)

Comment 12

4 years ago
Let me push a new one with for opt win32
(Assignee)

Comment 13

4 years ago
https://tbpl.mozilla.org/?tree=Try&rev=6bda2ae2bcfc
(Reporter)

Comment 14

4 years ago
(In reply to Olli Pettay [:smaug] from comment #13)
> https://tbpl.mozilla.org/?tree=Try&rev=6bda2ae2bcfc
Thanks!

I have tested the API, couple of questions:

1) getBoundMutationObservers returns list of observers for given element, but if an observer is registered for a parent element (with config.subtree=true) it isn't listed. Example:

<div id="content">
    <div id="testElement">Observe Me!</div>
</div>

If I register an observer for the 'content' element and call getBoundMutationObservers() on the 'testElement' the return value is empty. I think it should return all observers available in the parent chain. Or at least those, which have subtree=true...?

2) getObservingInfo returns the info, but why it's a list? There can be more info structures for one observer?

Honza
(Assignee)

Comment 15

4 years ago
(In reply to Jan Honza Odvarko from comment #14)
> 1) getBoundMutationObservers returns list of observers for given element,
> but if an observer is registered for a parent element (with
> config.subtree=true) it isn't listed. Example:
> 
> <div id="content">
>     <div id="testElement">Observe Me!</div>
> </div>
> 
> If I register an observer for the 'content' element and call
> getBoundMutationObservers() on the 'testElement' the return value is empty.
> I think it should return all observers available in the parent chain. Or at
> least those, which have subtree=true...?
definitely not without subtree true, since then changes to testElement don't have anything to do with
obverser registered on 'content'
And to keep the API simple, I'd prefer to not rely on subtree=true - or otherwise we should probably also
handle transient observers somehow. 


> 2) getObservingInfo returns the info, but why it's a list? There can be more
> info structures for one observer?
Yes. One per node passed to observe()
(Reporter)

Comment 16

4 years ago
OK, all make sense to me.

Honzas
Comment on attachment 800273 [details] [diff] [review]
patch + ad hoc tests

Review of attachment 800273 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/webidl/MutationObserver.webidl
@@ +28,5 @@
>    void disconnect();
>    sequence<MutationRecord> takeRecords();
> +
> +  [ChromeOnly]
> +  sequence<MutationObservingInfo?> getObservingInfo();

Why is this nullable? You never set it to null. Please change this to sequence<MutationObservingInfo>.

@@ +47,5 @@
>  };
> +
> +dictionary MutationObservingInfo : MutationObserverInit
> +{
> +  Node? observedNode = null;

Again, why nullable? Change to just Node.
Attachment #800273 - Flags: review?(peterv) → review+
(Assignee)

Comment 18

4 years ago
The reason is that our codegen can't handle those cases without nullable, and I didn't
want to fight with codegen just to get this done.
(Assignee)

Comment 19

4 years ago
Ah, maybe it can deal with always non-null observedNode
(Assignee)

Comment 20

4 years ago
Yes, that looks possible, but sequence<MutationObservingInfo> getObservingInfo(); isn't supported atm.
(Assignee)

Comment 21

4 years ago
Node observedNode is also a bit ugly to handle on C++ side, but I do it anyway.
(Assignee)

Comment 22

4 years ago
Actually, handling observedNode in C++ is just too ugly if there isn't ?, so I'll keep the ?.
(Assignee)

Comment 23

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/050debae43a6
https://hg.mozilla.org/mozilla-central/rev/050debae43a6
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Keywords: dev-doc-needed
(Reporter)

Comment 25

4 years ago
Olli, I am experiencing one problem. I need to find the script an observer.mutationCallback (a JS function) is defined in.

The |observer| is what I am getting from target.getBoundMutationObservers();

I am using JSD service:

var jsd = Cc["@mozilla.org/js/jsd/debugger-service;1"].getService(Ci.jsdIDebuggerService);
var wrapped = jsd.wrapValue(observer.mutationCallback);
var script = wrapped.script;

But I am getting an exception when executing the |jsd.wrapValue| API:
Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [jsdIDebuggerService.wrapValue]

Am I doing something wrong?
Or should I report a bug?

Honza
(Assignee)

Comment 26

4 years ago
Hmm, I don't know where that error is coming. Do you get a valid value from observer.mutationCallback? I assume yes.
If I read the code correctly, http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_xpc.cpp#2939 throws http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_xpc.cpp#59
I'm not familiar with JSD, so I don't know what that !mCx means. Are you failing to initialize JSD somehow?
(Reporter)

Comment 27

4 years ago
(In reply to Olli Pettay [:smaug] from comment #26)
> Hmm, I don't know where that error is coming. Do you get a valid value from
> observer.mutationCallback? I assume yes.
> If I read the code correctly,
> http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_xpc.cpp#2939 throws
> http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_xpc.cpp#59
> I'm not familiar with JSD, so I don't know what that !mCx means. Are you
> failing to initialize JSD somehow?
The problem was on my side (and actually related to initialization, good call)
Thanks
Honza
You need to log in before you can comment on or make changes to this bug.