677463 - <menuitem>.label should return .textContent if there's no label content attribute

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jan Varga [:janv]]

This is a followup bug from 617528.
Just in case the <menuitem> element won't become void element (bug 676236 and
http://www.w3.org/Bugs/Public/show_bug.cgi?id=13608)

For now, we have to use this syntax:
<menu>
  <menuitem label="Label"></menuitem>
</menu>

it would be nice to support also:
<menu>
  <menuitem>Label</menuitem>
</menu>

Comment 1

[Jan Varga [:janv]]

patch coming...

Comment 2

[Jan Varga [:janv]]

Attachment: patch v1

Comment 3

[Mounir Lamouri (:mounir)]

Is there any reason why we don't want to keep consistency with option.label?

Comment 4

[Jan Varga [:janv]]

do you mean the compress whitespace feature ?

Comment 5

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

Yeah, I was proposing to handle menuitem.label like option.label.
The patch adds some tiny inconsistencies.
(But I don't think we need to add .text to the public menuitem API)

Comment 6

[Jan Varga [:janv]]

ok, I'll redo it

Comment 7

[Jan Varga [:janv]]

Attachment: patch v2

Comment 8

[Jan Varga [:janv]]

Comment on attachment 552034 [details] [diff] [review]
patch v2

>+    <menuitem>Item w/ textContent</menuitem>

I put a space before "Item w/ textContent" to improve automatic testing.


>   return rv;
> }
> 
>-
> NS_IMPL_ENUM_ATTR_DEFAULT_VALUE(nsHTMLMenuItemElement, Type, type,

I put back the empty line.

Comment 9

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

Comment on attachment 552034 [details] [diff] [review]
patch v2

Would be great if you could add still some tests
which make sure that option.label handling stays the same
as menuitem.label.

Something like
var o = document.createElement("option");
var m = document.createElement("menuitem");
is(o.label, m.label, "Should have same labels");
o.textContent = "   ";
is(o.label, m.label, "Should have same labels");
o.textContent = "foo";
isnot(o.label, m.label, "Shouldn't have same labels");
m.textContent = "foo";
is(o.label, m.label, "Should have same labels");
m.label = "bar";
isnot(o.label, m.label, "Shouldn't have same labels");
o.label = "bar";
is(o.label, m.label, "Should have same labels");

Comment 10

[Jan Varga [:janv]]

sure, I'll add it

Comment 11

[Jan Varga [:janv]]

http://hg.mozilla.org/mozilla-central/rev/7871abb0e291

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Jan, is this bug fixed?

Comment 13

[Jan Varga [:janv]]

yes, fixed

Comment 14

[Eric Shepherd [:sheppy]]

Documentation updated:

https://developer.mozilla.org/en/XUL/menuitem
https://developer.mozilla.org/en/XUL/Attribute/label

And updated Firefox 8 for developers.

Comment 15

[Jan Varga [:janv]]

uhm, this bug is about html <menuitem>, not the xul one

Comment 16

[Eric Shepherd [:sheppy]]

Oops. Undoing that stuff, will try again. :)

Comment 17

[Eric Shepherd [:sheppy]]

I'm confused by this. There's no <menuitem> element in the specification, so where does this come from?

Comment 18

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

We think that the spec should have menuitem.
http://www.w3.org/Bugs/Public/show_bug.cgi?id=13608

If you document this, make sure to mention that spec doesn't have menuitem.

Comment 19

[Eric Shepherd [:sheppy]]

Removing doc-needed; documentation for this will be tracked by bug 676236 instead.