Implement disabled and checked attribute for menuitem.

Implement disabled attribute for menuitem.

The disabled attribute is a boolean attribute that, if present,
indicates that the command is not available in the current state.

Specification
https://html.spec.whatwg.org/multipage/forms.html#attr-menuitem-disabled


BUG=87553
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=186098

[pals, 2014-11-26 12:43:34]

Patchset #1 (id:1) has been deleted

[pals, 2014-11-26 12:43:55]

sanjoy.pal@samsung.com changed reviewers:
+ tkent@chromium.org

[pals, 2014-11-26 12:44:13]

PTAL.

[tkent, 2014-11-27 01:11:50]

Did you confirm |disabled| attribute disabled an item and we couldn't select the
item manually?  Unfortunately, I think we can't make an automated test for this
behavior.

Do you implement the following activation behavior defined in the specification?
 Without it, |checked| attribute doesn't make sense.

--------------------------
If the type attribute is in the Checkbox state
If the element has a checked attribute, the UA must remove that attribute.
Otherwise, the UA must add a checked attribute, with the literal value
"checked".

If the type attribute is in the Radio state
If the element has a parent, then the UA must walk the list of child nodes of
that parent element, and for each node that is a menuitem element, if that
element has a radiogroup attribute whose value exactly matches the current
element's (treating missing radiogroup attributes as if they were the empty
string), and has a checked attribute, must remove that attribute.

Then, the element's checked attribute must be set to the literal value
"checked".
--------------------------

IMO, there is no reason to implement |disabled| and |checked| in a single CL.
Probably we should have the following CLs:
1. Implement |disabled| attribute
2. Implement type=checkbox, including |checked| attribute
3. Implement type=radio, including |checked| and |radiogroup| attribute

[pals, 2014-11-27 06:57:19]

On 2014/11/27 01:11:50, tkent wrote:
> Did you confirm |disabled| attribute disabled an item and we couldn't select
the
> item manually?  Unfortunately, I think we can't make an automated test for
this
> behavior.

I confirmed in Linux desktop chromium, "disabled" and "checked" works as
expected.

> 
> Do you implement the following activation behavior defined in the
specification?
>  Without it, |checked| attribute doesn't make sense.

I will dot it in separate CL as you have suggested.

> 
> --------------------------
> If the type attribute is in the Checkbox state
> If the element has a checked attribute, the UA must remove that attribute.
> Otherwise, the UA must add a checked attribute, with the literal value
> "checked".
> 
> If the type attribute is in the Radio state
> If the element has a parent, then the UA must walk the list of child nodes of
> that parent element, and for each node that is a menuitem element, if that
> element has a radiogroup attribute whose value exactly matches the current
> element's (treating missing radiogroup attributes as if they were the empty
> string), and has a checked attribute, must remove that attribute.
> 
> Then, the element's checked attribute must be set to the literal value
> "checked".
> --------------------------
> 
> IMO, there is no reason to implement |disabled| and |checked| in a single CL.
> Probably we should have the following CLs:
> 1. Implement |disabled| attribute
> 2. Implement type=checkbox, including |checked| attribute
> 3. Implement type=radio, including |checked| and |radiogroup| attribute

Yes, I'll follow the above sequence. I have below plan to complete the menuitem.
> 1. Implement |disabled| attribute
> 2. Implement type=checkbox, including |checked| attribute
> 3. Implement type=radio, including |checked| and |radiogroup| attribute
Need to investigate if platform context menu supports type=radio. I haven't come
across contextmenu having radio in chrome. 
> 4. Implement Icon attribute. Working on it.
Fix issues
http://code.google.com/p/chromium/issues/detail?id=412945
http://code.google.com/p/chromium/issues/detail?id=415889
Also make necessary changes to allow type="context" with a warning to match
firefox.

[tkent, 2014-11-27 08:20:08]

The CQ bit was checked by tkent@chromium.org

[tkent, 2014-11-27 08:20:09]

lgtm

[commit-bot: I haz the power, 2014-11-27 08:20:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/761743002/40001

[tkent, 2014-11-27 08:24:14]

On 2014/11/27 06:57:19, sanjoy_pal wrote:
> Need to investigate if platform context menu supports type=radio. I haven't
come
> across contextmenu having radio in chrome. 

I don't think type=radio should have its own appearance.  The difference between
type=checkbox and type=radio is only activation behavior.

[commit-bot: I haz the power, 2014-11-27 09:33:01]

Committed patchset #2 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=186098