We might need this in bug 589146.

(It's also just nicer and more sane markup. Adding a binding was originally requested in bug 571842.)

Comment on attachment 491476 [details] [diff] [review] patch >+.split-menuitem-menu { >+ /* cut off inheritence */ >+ list-style-image: none; >+ -moz-image-region: auto; >+} Actually, this isn't needed, as menus don't inherit that stuff by default.

Maybe fodder for a followup, but it would sure be nice if the <splitmenu> could figure out its own label/key/command based on the first <menuitem> inside the popup. EG, the markup would just be: <splitmenu id="blah"> <menupopup> <menuitem label="foo" key="bar" command="baz"/> </menupopup> </splitmenu> (Faaborg says the corrent usage of splitmenus will always be to have the splitmenu and 1st menupopup menuitem look/work the same way.)

>(Faaborg says the corrent usage of splitmenus will always be to have the >splitmenu and 1st menupopup menuitem look/work the same way.) sometimes they will have a different string (for instance, in the Firefox menu we use "Bookmarks" but with "Show all Bookmarks" as the first item), but yeah, I think we are going to try to have the first item been the same action so that users can continue forward and never have to backtrack. It's redundant, but seems to significantly reduce confusion and streamline the user choosing between multiple items.

I think it's worth thinking about how we're going to solve the problems in bug 589146 before taking this patch, especially since it seems like that's what motivated this change. I tried working with this patch, but the new binding still makes it difficult to achieve the hover behavior described in bug 589146. I posted a WIP patch in that bug that uses a different binding to implement the hover effect, and although that patch still has problems, it seems to get closer to the right behavior than my attempts to use this binding.

(In reply to comment #6) I don't think waiting with this is really helpful. Bug 589146 can update the binding with whatever may be needed.

(In reply to comment #3) > Created attachment 491493 [details] [diff] [review] > patch I have found an issue that is caused by this patch. Some of the sub-menu items, particularly those on the right hand panel now seem to perform 2 actions. For example, selecting options and re-enabling the Menubar results in both the menubar being enabled and the Options dialog box opening. Similarly, doing a help -> about results in the about dialog and the help page being displayed.

I found that setting a custom attribute on the splitmenu element instead of command/oncommand fixes the problem Bill discovered.

with a simpler fix for the issue mentioned in comment 8

Comment on attachment 493922 [details] [diff] [review] patch v2 Seems like the binding should really just live over in toolkit's menu.xml. That's where I'd guess it would be (certainly not urlbarBindings.xml ;). r+ with that.

(In reply to comment #11) > Comment on attachment 493922 [details] [diff] [review] > patch v2 > > Seems like the binding should really just live over in toolkit's menu.xml. The reason I didn't put it in toolkit is that a) it doesn't work for the native mac menu and b) it lacks keyboard access (currently not an issue for the app button, as that can't be opened with the keyboard). I don't think this is ready for arbitrary consumers. > That's where I'd guess it would be (certainly not urlbarBindings.xml ;). r+ > with that. urlbarBindings.xml has become the dumping ground for custom browser.xul bindings. We could rename it, but that would affect extensions extending the bindings...

I am verifying that this fixes my issue form comment 8. The patch no longer applies cleanly because of bug 610922. Merely editing the patch file replacing all occurrences of preferencesCmd.label with preferencesCmd2.label is sufficient to get it to apply to current tip.

Comment on attachment 493922 [details] [diff] [review] patch v2 blocks a blocker, so I'm going to just land this

This broke the Menu Button History->Expand to view sub-menu Note that Recently closed tabs/windows is now just a small circle. Menu Item for History is OK

Filed: https://bugzilla.mozilla.org/show_bug.cgi?id=616461