it appears to be impossible to set an appropriate attribute in an RDF source and have it come out as a checkmark in XUL. It seems that existing attempts to set the attribute checked="rdf:http://home.netscape.com/NC-rdf#Checked" to "true" end unsuccessfully. The checkmark typically has to be set "manually" from JS. http://lxr.mozilla.org/seamonkey/source/intl/uconv/src/nsCharsetMenu.cpp#622 http://lxr.mozilla.org/seamonkey/source/xpfe/components/search/resources/search- panel.js#311

This is a generic problem with menus; specifically, a <menupopup> subtree with <menuitem type="radio"> built using the DOM APIs doesn't seem to honor the checked attribute. I'll attach a test case that demonstrates.

If you run the attached test case in viewer, and ``Dump Content'', you can see that the content model appears correct.

Nominating for nsbeta1 (for the intl bug 73881).

nhotta, waterson, it seems like the original test case was missing menuitem.setAttribute("type", "radio"). I don't think there is a problem with building a menu the DOM API. At least not as originally outlined. I've attempted to enhance the test case to better illustrate the problem I filed the bug for, however I don't seem to be able to access the charset menu RDF source when running inside the browser content window (see latest attachment). Maybe I can at least briefly outline the addition I had in mind. I wanted to set the checkmark in an RDF-based menu by adding an assertion to the RDF graph. In my private build, I replaced "menuitem.setAttribute('checked', 'true');" in UpdateCurrentMailCharset() from charsetOverlay.js with this code snippet: var rdf = Components.classes["@mozilla.org/rdf/rdf-service;1"].getService(Components.interfaces.nsIRDFService); var localStore = rdf.GetDataSource("rdf:local-store"); var charsetMenuSRC = rdf.GetResource("NC:ComposerCharsetMenuRoot", true); var checkedProperty = rdf.GetResource("http://home.netscape.com/NC-rdf#checked", true); var detectorID = menuitem.getAttribute("id"); var detectorSRC = rdf.GetResource(detectorID, true); localStore.Assert(charsetMenuSRC, checkedProperty, detectorSRC, true); The idea was that this should be equivalent with setting the checkmark via the DOM API, but then I might be missing something...

Oh brother, what an idjit! Yes, my DOM test case was broken, so there's no XPMenu bug here. I'll take the bug back, but am still not convinced the problem is in the toolkit. I'll attach yet another version of the menu bar that has an RDF-generated menu to illustrate...

taking from pink

jbetak: a couple comments on your test case. 1. The XUL <template> looks fine. By that I mean, it will ask the datasource for the |http://home.netscape.com/NC-rdf#checked| property, and set the |checked| attribute. 2. A checkmark (or dot) only appears on a XUL <menuitem> if the |checked| attribute is set to |true|. In other words, <menuitem type="radio" checked="blarg" /> will not do anything. 3. The JS that you wrote will create the following link in the RDF graph, which I don't think is what you want: [NC:ComposerCharsetMenuRoot] | +--[http://home.netscape.com/NC-rdf#checked]--+ | [some random charset resource]<------------+ The reason that I think this isn't what you want is that you're trying to match a pattern like _this_ in your template: [some random charset resource] | +--[http://home.netscape.com/NC-rdf#checked]--> "true" So, I'd suggest that you write your JS this way: var rdf = Components.classes["@mozilla.org/rdf/rdf-service;1"]. getService(Components.interfaces.nsIRDFService); var ls = rdf.GetDataSource("rdf:local-store"); // where |menuitem| is presumably the RDF-generated menu that we // want to select... var detectorID = menuitem.getAttribute("id"); ls.Assert(rdf.GetResource(detectorID), rdf.GetResource("http://home.netscape.com/NC-rdf#checked"), rdf.GetLiteral("true"), true); Alternatively, we could re-write the XUL template so that it would, in fact, match a structure like the JS you wrote is trying to create. What do you think?

(Restoring title to its original wording; however, the test case of 05/08/01 22:01 is a counter-example that illustrates that the XUL template builder is in fact generating a menu from RDF/XML that properly sets the |checked| state.)

waterson, now it's my turn to say !!??!? - my bad this time. I tried the suggested modification and somehow that still didn't yield the expected result. I also checked to see, what Cata originally wrote in nsCharsetMenu.cpp and I believe it matches your last comment: res = Assert(node, kNC_Checked, checkedLiteral, PR_TRUE); http://lxr.mozilla.org/seamonkey/source/intl/uconv/src/nsCharsetMenu.cpp#620 If memory serves, Cata was not able to get this to work and then came up with the kludge in UpdateCurrentMailCharset(), which recently fell apart. Would you have an idea, what's still going astray?

Adding meyself and ftang to this bug. 73881 depends on this one.

waterson: I used your RDF test menu and tried something I've attempted with the charset menu a while back: localStore.HasAssertion(rdf.GetResource("Choice 1"), rdf.GetResource("http://home.netscape.com/NC-rdf#checked"), rdf.GetLiteral("true"), true); This assertion query returns 'false' and localStore.Assert(rdf.GetResource("Choice 2"), rdf.GetResource("http://home.netscape.com/NC-rdf#checked"), rdf.GetLiteral("true"), true); doesn't move the checkmark from the first to the second item. Although apparently the XUL template, which was Tao's and mine prime suspect, seems to be fine, something is still not working right. Would you have an idea, where to look?

Okay, a couple of comments. First, on your test: localStore.HasAssertion(rdf.GetResource("Choice 1"), rdf.GetResource("http://home.netscape.com/NC-rdf#checked"), rdf.GetLiteral("true"), true); This isn't going to find an assertion, because the graph from my test case looks like this: [urn:root] | +--[http://www.w3.org/1999/02/22-rdf-syntax-ns#instanceOf] | | | v | [http://www.w3.org/1999/02/22-rdf-syntax-ns#Seq] | | +--[http://www.w3.org/1999/02/22-rdf-syntax-ns#_1]-->[-] | | | "Choice 1"<--[http://home.netscape.com/NC-rdf#name]-+ | | | "true"<--[http://home.netscape.com/NC-rdf#checked]-+ | | +--[http://www.w3.org/1999/02/22-rdf-syntax-ns#_2]-->[-] | | | "Choice 2"<--[http://home.netscape.com/NC-rdf#name]-+ | | +--[http://www.w3.org/1999/02/22-rdf-syntax-ns#_3]-->[-] | "Choice 3"<--[http://home.netscape.com/NC-rdf#name]-+ Where [-] connotates an anonymous resource that's created for a <Description> element without an |ID| or |about| attribute. So, your call to |rdf.GetResource("Choice 1")| is getting a node that's not really in this graph. We can talk about this more off-line if you're interested. Second, the code you referred me to in nsCharsetMenu.cpp. I think there's a bug in |nsCharsetMenu::SetCharsetCheckmark()|. Specifically, that method generates a resource URI as follows: char csID[256]; aCharset->ToCString(csID, sizeof(csID)); res = mRDFService->GetResource(csID, getter_AddRefs(node)); However, in |nsCharsetMenu::AddMenuItemToContainer()|, it uses more complicated logic: nsAutoString cs; res = aItem->mCharset->ToString(cs); if (NS_FAILED(res)) return res; nsAutoString id; if (aIDPrefix != NULL) id.AssignWithConversion(aIDPrefix); id.Append(cs); // Make up a unique ID and create the RDF NODE char csID[256]; id.ToCString(csID, sizeof(csID)); res = mRDFService->GetResource(csID, getter_AddRefs(node)); So, are we sure that |aIDPrefix| is always null? Without tracing through things all that carefully, I see several cases where it is not.

(Sigh, and I can't bring up the context menu to test this. Adding dependency to bug 78725.)

I've investigated this a bit more. The problem appears to have been caused by a re-ordering of the menu's |oncreate| handler and the time at which RDF is building the content. Specifically, the |oncreate| handler is firing before RDF gets a chance to build any children. (This may not be a bad thing; maybe you'd want to change the `ref' attribute or something.) Here's a nasty hack in charsetOverlay.js that will fix the immediate problem for the main charset menu; other menus would need to be hacked similarly. function UpdateMenus(event) { - UpdateCurrentCharset(); + setTimeout("UpdateCurrentCharset();", 0); UpdateCharsetDetector(); } A more elegant fix might be to add an |oncreatecomplete| event. Maybe we could create an RFE on XPToolkit for that. Kicking back to intl.

hyatt, pink: any thoughts on an |oncreatecomplete| event for XP menus? (See above.)

waterson- the reason you kick back to intl is to ask use to put the temp hack for now, right ? Should we 1. put your hack into our charset menu code and check in, 2. reassign this bug back to you to work on complete fix later?

Changing QA contact to teruko for now, please re-assign further as appropriate.

Either way, we're going to need to change the intl code. I'll talk to pink and hyatt today to see how they feel about an |onmenucomplete| event.

I tried the setTimeout() workaround and it seems to work. But I am not sure how this make it work. waterson, why setTimeout("UpdateCurrentCharset();", 0); fix the problem?

Here is what happens when a menu is created: 1. User clicks on the menu 2. Menu's |oncreate| event handler is run. 3. RDF is told to construct menu contents By doing a setTimeout(), you're scheduling an event that occurs in a callback, which fires after RDF has constructed the menuitems. So there are a couple of ways that we could fix this: 1. Change the ordering so that |oncreate| fires after RDF constructs the menus. 2. Leave |oncreate| alone, and add a new event, |oncreatecomplete|, that fires after RDF has constructed its content. 3. Use |setTimeout| to simulate (2). I don't think we should change the ordering, because I'm suspicious that some people may depend on having |oncreate| run _before_ RDF so that they can change what RDF builds. hyatt and pink seem amicable to creating an |onmenucomplete| event (or something like it; they're planning to rename all the event handlers for XUL 1.0), which would be the ``cleanest'' way to solve the problem. Certainly using |setTimout| would be the most expedient way to get this stuff working; we could go back and clean up that code when we're changing over to the new menu event syntax. So, I guess I'd recommend (3) for now, and then file a FUTURE bug for using the |onmenucomplete| event.

setTimeout won't work on macos, as we're deep w/in OS code and not processing the event loop.

The charset menu checkmark problem (bug 73881) does not happen on Macintosh somehow.

Filed bug 82025, requesting for onmenucomplete.

Clear the milestone since we have a workaround patch in bug 73881. Reassign to waterson since this bug is not i18n specific.

I hope it is somehow related : In addition to never been able to used the checked attribute, I never successfuly (with Mozilla 1.4) make work those kind of xul/rdf code : <vbox id="algorithm" flex="1" datasources="blabla.rdf" ref="urn:root"> <template> <groupbox flex="1" uri="rdf:*"> <caption label="rdf:http://ltrim.com/AGO-rdf#label"/> <radiogroup selectedIndex="rdf:http://ltrim.com/AGO-rdf#select"> <radio label="Simple"/> <radio label="Steady State"/> <radio label="Incremental"/> <radio label="Deme"/> </radiogroup> </groupbox> </template> </vbox> The caption shows me that I am not so fool... Thanks

Updated test case (url of attachment changed)

The updated test case is WFM.

> The updated test case is WFM. I removed the setTimeout in Bug 73881 and the problem is still there so the test case doesn't cover all cases.