Implement type=radio and radiogroup attribute for menuitem.

Source/core/html/HTMLMenuItemElement.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/html/HTMLMenuItemElement.h"
 
 #include "core/HTMLNames.h"
+#include "core/dom/ElementTraversal.h"
 #include "core/events/Event.h"
 
 namespace blink {
 
 using namespace HTMLNames;
 
 inline HTMLMenuItemElement::HTMLMenuItemElement(Document& document)
     : HTMLElement(HTMLNames::menuitemTag, document)
 {
 }
 
 void HTMLMenuItemElement::defaultEventHandler(Event* event)
 {
     if (event->type() == EventTypeNames::click) {
         if (equalIgnoringCase(fastGetAttribute(typeAttr), "checkbox")) {
             if (fastHasAttribute(checkedAttr))
                 removeAttribute(checkedAttr);
             else
                 setAttribute(checkedAttr, "checked");
+        } else if (equalIgnoringCase(fastGetAttribute(typeAttr), "radio")) {
+            if (Element* parent = parentElement()) {
+                String group = fastHasAttribute(radiogroupAttr) ? fastGetAttribute(radiogroupAttr) : "";

[tkent, 2014/12/10 06:50:49]

This code searches the element for radiogroupAttr twice.
The return value should be |const AtomicString&|.

We should do just:

const AtomicString& group = fastGetAttribute(radiogroupAttr);

|group| is a null AtomicString if the element has no radiogroup attribute.

[pals, 2014/12/10 07:11:46]

Done.

+                for (HTMLMenuItemElement& menuItem : Traversal<HTMLMenuItemElement>::childrenOf(*parent)) {
+                    if (equalIgnoringCase(menuItem.fastGetAttribute(typeAttr), "radio") && menuItem.fastHasAttribute(checkedAttr)) {

[tkent, 2014/12/10 06:50:49]

Don't check type=radio here.  The specification doesn't ask to check if
type=radio.

We prefer early continue.
    if (!menuItem.fastHasAttribute(checkedAttr))
        continue;

[pals, 2014/12/10 07:11:46]

But, this looks like a specification bug. I shall check with specification.
Done.

+                        String groupAttr = menuItem.fastHasAttribute(radiogroupAttr) ? menuItem.fastGetAttribute(radiogroupAttr) : "";

[tkent, 2014/12/10 06:50:49]

Don't need to call fastHasAttribute like line. 31.

[pals, 2014/12/10 07:11:46]

Done.

+                        if (groupAttr.stripWhiteSpace() == group.stripWhiteSpace())

[tkent, 2014/12/10 06:50:49]

Don't use stripWhiteSpace().  The specification doesn't ask to do so.

equalIgnoringNullity(group.impl(), groupAttr.impl())

should be enough.

[pals, 2014/12/10 07:11:46]

Done.

+                            menuItem.removeAttribute(checkedAttr);
+                    }
+                }
+            }
+            setAttribute(checkedAttr, "checked");
         }
         event->setDefaultHandled();
     }
 }
 
 DEFINE_NODE_FACTORY(HTMLMenuItemElement)
 
 } // namespace blink