399634 - <radiogroup persists="value"> fails when loaded dynamically via document.loadOverlay

Status: RESOLVED WONTFIX

(Core :: XUL, defect)

Description

[mv_van_rantwijk]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a6pre) Gecko/20070927 SeaMonkey/2.0a MultiZilla v1.8.3.3p
Build Identifier: 

The patch for https://bugzilla.mozilla.org/show_bug.cgi?id=371260
breaks <radiogroup persist="value" as in the following bits in the constructor:

          else
            this.selectedIndex = 0;

http://mxr.mozilla.org/mozilla/source/toolkit/content/widgets/radio.xml#31
Should I file a bug for this, or can someone explain to me how I can fix and get persist="value" working again? 

Replacing it with this:

          function _inner(aRadioGroupNode) {
            var value = aRadioGroupNode.value;

            if (value)
              aRadioGroupNode.value = value;
            else
              aRadioGroupNode.selectedIndex = 0;
          }
          setTimeout(_inner, 0, this);

makes persist happy again. 

Reproducible: Always

Comment 1

[mv_van_rantwijk]

About this: "Should I file a bug for this, or can someone explain to me how I can fix and get persist="value" working again?" that's here because I left it in after I asked for help/advise in the newsgroups.

Note: This is possibly related to bug 266590.

Comment 2

[[e-mail]]

Odd... ApplyPersistentAttributes() is called before StartLayout() so the value should already exist on line 28 and no timeout hack should be needed.

Comment 3

[mv_van_rantwijk]

(In reply to comment #2)
> Odd... ApplyPersistentAttributes() is called before StartLayout() so the value
> should already exist on line 28 and no timeout hack should be needed.

Normally that works, yes, but it fails when you use document.loadOverlay() like the preference window does (for example).

Comment 4

[[e-mail]]

document.loadOverlay reapplies all persistent attributes? /me runs screaming

Comment 5

[mv_van_rantwijk]

(In reply to comment #4)
> document.loadOverlay reapplies all persistent attributes? /me runs screaming

Are you saying that having attributes like 'persist' should be skipped/ignored when people use 'document.loadOverlay' which work when they use: 
<?xul-overlay href="chrome://some/content/someOverlay.xul"?> Again, this used to work!

Comment 6

[Nickolay_Ponomarev]

So issue #1 is that XBL constructors for dynamically overlaid content fire when the element in question is inserted in the DOM (therefore before the persisted attributes are set) and issue #2, unrelated to the original bug, is that

> document.loadOverlay reapplies all persistent attributes

[Re comment 5 - it probably should apply persistent attributes only to the content added by the overlay. Right now it looks like it re-sets all the attributes in localstore]. Issue #2 shouldn't be very visible, since iirc we keep localstore in sync with the actual values of the attributes.

Comment 7

[Nickolay_Ponomarev]

I don't see how it could work before though (and it doesn't work with the following code on fx2 for me):

<?xml version="1.0"?>
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
<window id="yourwindow" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
<radiogroup persist="value"><radio value="1" id="1" label="1"/><radio value="123" id="2" label="123"/></radiogroup>
</window>

Speaking of which, a testcase would be welcome.

Comment 8

[mv_van_rantwijk]

(In reply to comment #6)
> So issue #1 is that XBL constructors for dynamically overlaid content fire when
> the element in question is inserted in the DOM (therefore before the persisted
> attributes are set) and issue #2, unrelated to the original bug, is that
> 
> > document.loadOverlay reapplies all persistent attributes
> 
> [Re comment 5 - it probably should apply persistent attributes only to the
> content added by the overlay. Right now it looks like it re-sets all the
> attributes in localstore]. 

But of course. I agree.

> Issue #2 shouldn't be very visible, since iirc we
> keep localstore in sync with the actual values of the attributes.

The values in localstore are fine, yes, but fail to get reapplied.

(In reply to comment #7)
> I don't see how it could work before though (and it doesn't work with the
> following code on fx2 for me):

It worked up until the patch for the bug 371260 went in, and taking two lines out makes it work again.

Comment 9

[mv_van_rantwijk]

(In reply to comment #7)
> I don't see how it could work before though (and it doesn't work with the
> following code on fx2 for me):
> 
> <?xml version="1.0"?>
> <?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
> <window id="yourwindow"
> xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
> <radiogroup persist="value"><radio value="1" id="1" label="1"/><radio
> value="123" id="2" label="123"/></radiogroup>
> </window>

So where is your id (radiogroup) which you need to get persist working? 

Comment 10

[Nickolay_Ponomarev]

Ah, I knew it was something I forgot. Thanks.

Comment 11

[Andrew McCreight [:mccr8]]

XUL overlays were removed in bug 1426763.