Open Bug 676236 Opened 10 years ago Updated 2 years ago
Add support for the <menuitem> element to the HTML parser
We need this for bug 617528. We decided to replace <command> with <menuitem> The new syntax is: <menu type="context"> <menuitem type="" label="" ...> ... </menu> The behavior of <menuitem> is similar to <command>. However there is no need to use <menuitem> in <head> There are plans to upgrade the syntax in future to: <head> <command id="foo" type="" label="" ...> </head> <body> <menu type="context"> <menuitem command="foo"> </menu> </body>
Comment on attachment 550371 [details] [diff] [review] patch v1 (In reply to comment #0) > We need this for bug 617528. We decided to replace <command> with <menuitem> Decided? Why? Who is "we"? Why hasn't this been raised at the WHATWG or the W3C HTML WG? The days of browser vendors adding HTML parsing behaviors without coordinating through the spec are supposed to be over. It's painful (see www.w3.org/Bugs/Public/show_bug.cgi?id=13113) but coordinating through the spec is the right thing to do.
Attachment #550371 - Flags: review?(hsivonen) → review-
Before the menuitem gets spec'ed, we should #ifdef/pref the code, IMO.
Comment on attachment 550371 [details] [diff] [review] patch v1 r=me on the condition that the lines case MENUITEM: are #ifdefed off by default when landing on trunk ahead of the spec making <menuitem> a void element. (I'll figure out what to do about #ifdefs and the translator when I return from vacation.)
Attachment #550371 - Flags: review- → review+
I'm updating and testing a patch with the ifdefs
Assignee: nobody → Jan.Varga
Comment on attachment 550678 [details] [diff] [review] Patch with ifdef'ed code r=me assuming that the define continues to be off by default until the spec changes to have menuitem as a void element.
Attachment #550678 - Flags: review+
smaug told me that <menuitem> is no longer meant to be a void element. Can we back this out now?
Is this still in progress? I was under the impression we'd implemented this already, based on bug 677463. Can I get a clarification as to the status here before I document it? Thanks!
OK, when time comes to document this, I need to reference bug 677463 to be sure that behavior is covered.
According to  and the spec, menuitem is a void element. So the pref can be flipped now, I think. Anything else left to do in this bug?  http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2012-December/038472.html  http://www.whatwg.org/specs/web-apps/current-work/multipage/interactive-elements.html#the-menuitem-element
Well, yes, but it's not just the pref. All the tests need to be updated to use just <menuitem label="...">
initial patch (more work needs to be done)
Hm, trying to clear the needinfo flag, sorry for the spam.
It would be good to get input on this spec issue: https://github.com/whatwg/html/issues/234#issuecomment-146335204
Based on the link in comment #20, the spec ended up changing such that <menuitem> is not void, but is <option>-like (and no longer in the special category). smaug, I can't quite tell if that means Firefox now handles <menuitem> correctly, or if there are still differences which need to be addressed. Can you see anything obvious? At any rate I'm guessing we might as well close this bug as INVALID, and file a follow-up to remove the few lines of code which add the build option for making <menuitem> a void element.
https://github.com/whatwg/html/issues/234 is still open, so I don't really understand what has happened in the spec. I'd be ok to make us work closer to <option> if that is what the spec and other browsers will have too. If you want to do some parser changes, ask hsivonen to review ;)
Henri, could you take a look at whether something needs to be done here. The spec bug was closed but it is still a bit unclear to me what actually got changed. (some times I hate github making it so hard to follow changes)
The spec changed from void to parse "like <option>", with optional end tag. Processing model changes: https://github.com/whatwg/html/commit/8fdccb55e40a36449ae6011f9960cb09c1da0db7 Parser changes: https://github.com/whatwg/html/commit/5e49a20874fabef620bf7ea0be7534c73fbd58c4 https://github.com/whatwg/html/commit/2999aa01438bdef11ba10aef82b775293efbef79 HTH,
I'll take a look.
Do you think you'll have a chance to take a look at this anytime soon, hsivonen?
Looking now. See the previous comment for a try push.
Which other browsers have implemented this spec change?
None yet. http://software.hixie.ch/utilities/js/live-dom-viewer/saved/4740 https://bugs.chromium.org/p/chromium/issues/detail?id=412945 is the relevant issue for Chromium. It is implemented in https://github.com/inikulin/parse5 though.
These are now merged: https://github.com/whatwg/html/pull/2319 https://github.com/html5lib/html5lib-tests/pull/88
Per https://github.com/whatwg/html/issues/2730 this should probably be wontfixed.
https://github.com/whatwg/html/pull/2319 made menuitem parse like an unknown element. If there are ifdefs, I suppose they can be removed.
You need to log in before you can comment on or make changes to this bug.