342347 - <xul:key disabled="true" command="foo"/> doesn't honor disabled attribute

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Alex Vincent [:WeirdAl]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060621 SeaMonkey/1.5a

For accessibility purposes, I envisioned dynamically disabling and enabling key sets on the fly.  However, when I went to test this, I got some very startling results.

<xul:key disabled="true" oncommand="alert('foo');"/>
You'd never see the alert.

<xul:key disabled="true" command="foo"/>
<xul:command id="foo" oncommand="alert('foo');"/>
You would see the alert here.

Testcase coming up.

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: testcase

Comment 2

[James Ross]

According to documentation, 3 attributes are mirrored/redirected from the command element: oncommand, label and disabled. I don't know if the target (xul:key in this case) is allowed to override the command, though I can see cases for either way.

Comment 3

[Alex Vincent [:WeirdAl]]

I think it's okay for a particular key combination to be disabled while the original command is enabled.

<menuitem key="keyFoo" command="cmdFoo"/>

Comment 4

[Alex Vincent [:WeirdAl]]

Attachment: patch, v1

Comment 5

[Alex Vincent [:WeirdAl]]

cc'ing bryner - this affects code he has very recently touched.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 226542 [details] [diff] [review]
patch, v1

I think it is ok to not to dispatch a command event in the case, when disabled="true". in other xul elements when a click happens, command event is not dispatched if disabled="true".
For example
http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsButtonBoxFrame.cpp#148

Comment 7

[Alex Vincent [:WeirdAl]]

Comment on attachment 226542 [details] [diff] [review]
patch, v1

Seeking second review (should be super-review) from bryner.

Comment 8

[Alex Vincent [:WeirdAl]]

Fix checked in by smaug - thanks everyone!