The constructors and destructors of <radio> elements in XUL always assume that they are in a document and part of a <radiogroup>. If this assumption fails (which it regularly does) we get the following error for the constructor: Error: null has no properties Source File: chrome://global/content/bindings/radio.xml Line: 383 And that one for the destructor: Error: this.radioGroup has no properties Source File: chrome://global/content/bindings/radio.xml Line: 388 The line numbers above are for Firefox 2.0.0.1, this is an issue on trunk as well however. In Firefox 1.5 this is less of an issue since bindings are only applied when the node is inserted into a document - simply cloning a radio element won't cause these errors.

This testcase has a radio element without a radiogroup in the document. When it loads, another radio element is created using cloneNode() - not inserted into the document however. Both elements cause the errors mentioned above, and these errors cannot be caught.

This tests the functionality that the code in the constructor/destructor should provide but doesn't. We make sure that radiogroup.mRadioChildren is initialized by setting the value property. When we add another radio element mRadioChildren should be cleared - but it doesn't in this case, consequently setting value to the new element fails, also the alert with radiogroup._getRadioChildren().length shows 1 instead of the expected 2 children.

Moving the operations to update mRadioChildren into the radiogroup itself, triggered by DOMNodeInserted and DOMNodeRemoved events instead of XBL construction. Both testcases work with this change, also removing a radio element from the group removes it from mRadioChildren as well.

bz, do mutation events still send perf through the floor these days? trev, some thoughts: 1. Just unconditionally clear the cache, without checking the inserted node 2. Listen for the mutation events on the radio itself (bz: does this work?) 3. Write C++ code to special-case mutation events targeted on the element (i.e. without having to turn on mutation events globally).

Neil, the node itself doesn't receive mutation events, only its parent nodes. Looking at http://www.w3.org/TR/DOM-Level-3-Events/events.html#event-DOMNodeInserted I don't see why it should behave like this so this is probably a bug itself.

> bz, do mutation events still send perf through the floor these days? Good question. Sicking's cleaned them up some, but DOM event dispatch is pretty expensive. So I suspect that adding a listener for some operation does significantly slow down that operation. One could test to find the number, I suppose. > Listen for the mutation events on the radio itself (bz: does this work?) It should. If it doesn't, we should fix it. Last I checked this did work, though I wasn't testing XUL. Is there a testcase that shows it not working? Also, does it depend on whether a capturing handler is used (not that it should, in our impl; just checking)?

Mutation events are unfortunately still very slow, so we should try to avoid them.

This tests whether mutation events fire for the container and the child being added/removed. Actually, when we have listeners attached to the container the events fire for the child as well. If you comment out these listeners, the listeners for the child won't fire any more.

Indeed. Filed bug 367164 on that issue.

(In reply to comment #4) > trev, some thoughts: > 1. Just unconditionally clear the cache, without checking the inserted node > 2. Listen for the mutation events on the radio itself (bz: does this work?) > 3. Write C++ code to special-case mutation events targeted on the element > (i.e. without having to turn on mutation events globally). Sorry Neil, that will need more time than I can spend at the moment. Feel free to take this bug.

Comment on attachment 251502 [details] [diff] [review] Proposed patch Minusing as per comment #7.

(In reply to comment #4) > 3. Write C++ code to special-case mutation events targeted on the element > (i.e. without having to turn on mutation events globally). > Can you clarify this? I suppose http://lxr.mozilla.org/mozilla/source/content/base/src/nsContentUtils.cpp#2844 is too slowly, right? How it can be improved for special-case mutation events targeted on the element?

I mean that it may be possible for the XBL binding manager to synthetically trigger handlers even though mutation events haven't been enabled.

Yes, it should be possible to add code to XBL that implements some mutation events without registering a true mutation-event-handler (and thus triggering the global flag). I plan to do so for XBL2, but I haven't looked into how doable it is in the current XBL1 implementation.

Please note bug 209555, which I believe this is a dupe of.

Can you see if the issue was fixed via bug 209555 and we can close this?

I worked around that issue a long time ago but judging from the current radio.xml code the issue is indeed resolved. The relevant changes are bug 371260 and bug 606215, bug 209555 on the other hand addressed a different issue. So - WORKSFORME.