75516 - Support for new "command=" syntax does not handle "type=checkbox/radio" menu items

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

Bug 71470 added special support to menu items to support the new "command=" 
syntax.  This support copies the "disabled" and "value" attributes into the 
menu from the command.  However, this broke any menuitems 
with 'type="checkbox"' that use the command update mechanism, as the "checked" 
state does not work correctly.

There appear to be no menu items in the Mozilla code base that use the command 
mechanism to update their "checked=" state - all such menu items do not 
use "observes" (now "command"), but update the state directly.  However Komodo 
does use the command infrastructure for such menu items.

Attaching a patch that also copies the "checked" attribute from the command 
into the menu item before display.

Adding Hyatt and Pinkerton to the CC list as they seemed to be the main drivers 
in 71470 (sorry guys!)

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Attachment: Proposed patch that updates the "checked" attribute from the command.

Comment 2

[David Hyatt]

Mark, the patch is good.  YOu also need to patch the native Mac menu code over
in widget/src/mac.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Attachment: Updated patch that touches the Mac code. Note the mac code hasn't been compiled, let alone tested!

Comment 4

[Peter "jag" Annema]

The changes look okay, but what would this mean for Mozilla? It looks like we'd
need to move the checked state to the <command/>, otherwise the absence of it
will override what's in the <menuitem/>.

Or we need to treat it the way the value attribute is treated.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Jag is, as usual, correct :)  It did indeed break a couple of checkbox menus in 
Moz.  Attaching a new patch to treat it the way the value attribute is treated.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

Attachment: New patch that allows a menu to have its own value if the command doesn't.

Comment 7

[David Hyatt]

Fixed.

Comment 8

[Mark Hammond [:markh] [:mhammond]]

Works for me.  Thx!