27433 - No radio behavior when manipulating a radio menu from its RDF data source

Status: VERIFIED INVALID

(Core :: XUL, defect, P1)

Description

[cata]

I have a menu type="radio" created from an RDF datasource. I am expecting the 
existent checkmark to go away when I set a new checked item from CPP. Well, it 
does not.

Menu XUL code:

        <template>
            <rule>
	            <menupopup>
                <menuitem type="radio" name="charsetGroup" 
checked="rdf:http://home.netscape.com/NC-rdf#Checked" uri="..." 
value="rdf:http://home.netscape.com/NC-rdf#Name" 
oncommand="SetDefaultCharacterSet(event.target)"/>
	            </menupopup>
            </rule>
        </template>

Checking menu code:

  // set checkmark value
  nsCOMPtr<nsIRDFLiteral> checkedLiteral;
  nsAutoString checked((aValue == PR_TRUE) ? "true" : "false");
  res = rdfServ->GetLiteral(checked.GetUnicode(), 
getter_AddRefs(checkedLiteral));
  if (NS_FAILED(res)) return res;
  res = Assert(node, kNC_Checked, checkedLiteral, PR_TRUE);
  if (NS_FAILED(res)) return res;

Comment 1

[David Hyatt]

Giving to waterson to see if it's a template bug and to sanity-check the 
template stuff.

Comment 2

[bobj]

Bug 11774 depends upon this being resolved.

Comment 3

[Chris Waterson]

Ack. First off, do -not- put the event handler on the menu item. Put it in the
menu. Otherwise, you'll end up with an event handler being compiled and bound
once for every single menu item. Very expensive. Fix that, and give the bug back
to me if it doesn't work.

Comment 4

[cata]

Actually, I intend to have different handlers for menu items: I will have 2 
groups in the same menu. So, is there any other way to do this. Also, why is it 
a problem?! It sould still work, even if it is not efficient...

Here's the XUL template:
        <template>
            <rule rdf:type="http://home.netscape.com/NC-rdf#CharsetDetector">
	            <menupopup>
                <menuitem type="radio" name="detectorGroup" 
checked="rdf:http://home.netscape.com/NC-rdf#Checked" uri="..." 
value="rdf:http://home.netscape.com/NC-rdf#Name" 
oncommand="SelectDetector(event)"/>
	            </menupopup>
            </rule>
            <rule rdf:type="http://home.netscape.com/NC-rdf#BookmarkSeparator">
	            <menupopup>
                <menuseparator uri="..." />
	            </menupopup>
            </rule>
            <rule>
	            <menupopup>
                <menuitem type="radio" name="charsetGroup" 
checked="rdf:http://home.netscape.com/NC-rdf#Checked" uri="..." 
value="rdf:http://home.netscape.com/NC-rdf#Name" 
oncommand="SetDefaultCharacterSet(event.target)"/>
	            </menupopup>
            </rule>
        </template>

Comment 5

[Chris Waterson]

You are right, it should work. I suspect your being bitten by a variation of bug
26402. That said...

You are not allowed to do this :-).

Seriously, generating the charset menus currently accounts for something like
5-10% of startup time. That's very, very bad. So, tag your menu items (or
something similarly clever), and make your single event handler smart enough to
know the difference between the two. Treat this as an "opportunity" to fix the
event handler stuff! :-)

Comment 6

[cata]

You are right. I will fix it ASAP. Yeah, tagging should work, thanks for the 
idea.

But I'll still need for the bug to be fixed... Maybe it'll go away when I put 
the handler on <menu>? Maybe. I'll let you know.

Comment 7

[cata]

Bad news: after changing the menu the way you told me, with the handler on 
"menu", the problem is still there. :))) Unchanged. Any ideas?

Comment 8

[Chris Waterson]

Hmm. Let me understand this. The -newly- selected item is being checked; the 
-old- item is *not* being unchecked, correct?

If this is in fact the case, do you "unselect" the old one? e.g., by doing an 
Unassert() on the currently selected charset/detector?

(To pick nits: you should be able to do all of this in JS. Why are you dropping 
into native code?)

Comment 9

[leger]

Putting on the PDT+ radar for beta1.

Comment 10

[Chris Waterson]

Attachment: test case

Comment 11

[Chris Waterson]

cata: the attached test case WORKSFORME, so I'm pretty sure this is not a 
problem with RDF or menus per se. What are you doing differently? What happens 
if you remove your event handlers? Are your event handlers doing "too much"?

Comment 12

[cata]

Chris, your attached test case works for me as well. But the problem is not 
there. The RDF radio menus work ok as long as the cheking/unchecking is done 
from GUI interation. But when I set one of the menus checked from CPP code like 
this:

in XUL:

<menuitem type="radio" name="detectorGroup" 
checked="rdf:http://home.netscape.com/NC-rdf#Checked" uri="..." 
value="rdf:http://home.netscape.com/NC-rdf#Name"/>

in CPP:

Assert(node, kNC_Checked, checkedLiteral, PR_TRUE);

Then this newly checked item does not seem to belong to the original radio group 
anymore. It will be independent, not being unchecked when a new item is pressed.

I hope I explained the problem well. But please ring me and let's meet and I can 
show you on my pc what's going on. Thanks!

PS. I moved the handlers on the <MENU>

Comment 13

[Chris Waterson]

I'm resolving this bug as INVALID :-(

Here's why. What you are doing ends up being exactly the same as if you'd
written the following XUL:

 <menu>
   <menupopup>
      <menuitem type="radio" name="group" checked="true" value="one" />
      <menuitem type="radio" name="group" checked="true" value="two" />
   </menupopup>
 </menu>

In which case, the code is doing exactly what it is supposed to do. You need to
either:

1. Change your back-end code to Unassert() the old "checked --> 'true'" when
manipulating the datasource.

2. Re-implement this to let XUL do the work for you; e.g., set the selection
using a JS onload handler, and then let your menu's oncommand twiddle with the
back-end.

The RDF content model builder *only* copies data through from the front-end to
the back end. And, in every toolkit I have ever used, setting multiple radio
buttons as "selected" has caused them *all* to be selected. (Unless you go
through the "click" or "choose" API, which the RDF builder just cannot do.)

Sorry! :-(

Comment 14

[cata]

Ok, I'm attempting to go with the JS solution, but I don't have any docs! Can 
you please provide me with a short example? Like where should I place the onload 
handler? It is being ignored on <menu> or <menupopup> ...

Comment 15

[sairuh (rarely reading bugmail)]

hokay, am verifying this...but if this is still an issue, pls reopen.