Closed Bug 912874 Opened 11 years ago Closed 11 years ago

New API to enumerate mutation observers

Categories

(Core :: DOM: Events, defect)

Product:
Core
Component:
DOM: Events
Version:
20 Branch
Platform:
x86
Windows Vista
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla26

People

(Reporter: Honza, Assigned: smaug)

Details

(Keywords: dev-doc-needed)

Attachments

(1 file)

patch + ad hoc tests
18.58 KB, patch
peterv
: review+
Details | Diff | Splinter Review 
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
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?
Or perhaps Node should just have
[ChromeOnly] sequence<MutationObserver> getAttachedMutationObservers();

And MutationObserver
[ChromeOnly] sequence<MutationObservingInfo> getObservingInfo();
and [ChromeOnly] readonly attribute MutationCallback callback;
(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
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
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.
Or would you need notification only about explicit removal? Hmm, though, then the API would be just inconsistent.
(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
Patch coming
Assignee: nobody → bugs
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)
Wow, that was quick!

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

Honza
Let me push a new one with for opt win32
(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
(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()
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+
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.
Ah, maybe it can deal with always non-null observedNode
Yes, that looks possible, but sequence<MutationObservingInfo> getObservingInfo(); isn't supported atm.
Node observedNode is also a bit ugly to handle on C++ side, but I do it anyway.
Actually, handling observedNode in C++ is just too ugly if there isn't ?, so I'll keep the ?.
https://hg.mozilla.org/mozilla-central/rev/050debae43a6
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
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
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?
(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.

Attachment

General

Creator:
Created:
Updated:
Size: