Implement HTMLMenuItemElement.label

third_party/WebKit/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 "core/html/HTMLMenuItemElement.h"
 
+#include "core/html/parser/HTMLParserIdioms.h"
 #include "core/HTMLNames.h"
 #include "core/dom/ElementTraversal.h"
 #include "core/events/Event.h"
 #include "core/frame/UseCounter.h"
 
 namespace blink {
 
 using namespace HTMLNames;
 
 inline HTMLMenuItemElement::HTMLMenuItemElement(Document& document)
@@ skipping 33 matching lines @@
           if (EqualIgnoringNullity(group_attr.Impl(), group.Impl()))
             menu_item.removeAttribute(checkedAttr);
         }
       }
       setAttribute(checkedAttr, "checked");
     }
     event->SetDefaultHandled();
   }
 }
 
+const String HTMLMenuItemElement::label() const {

[tkent, 2017/04/25 23:25:00]

The return value should not be |const|.  It's unnecessary constraint.

[yuzuchan, 2017/04/26 08:54:57]

Right! Done.

+  const AtomicString label = FastGetAttribute(labelAttr);
+  if (!label.IsNull() && label != "")

[tkent, 2017/04/25 09:56:34]

if (!label.IsEmpty())

is simpler.

[tkent, 2017/04/25 23:24:59]

I found the specification didn't ask to check an empty attribute value. This
should be |if (!label.IsNull())|.

https://html.spec.whatwg.org/multipage/forms.html#dom-menuitem-label
So |label| IDL attribute value and |label| concept are different if |label|
content attribute is empty.  So confusing!

Also, we should update CustomContextMenuProvider::AppendMenuItem() so that it
refers to label IDL attribute value or label concept instead of label content
attribute.

[yuzuchan, 2017/04/26 08:54:57]

Changed the code to follow the spec. Thanks.

+    return label;
+  return this->textContent(true).StripWhiteSpace(IsHTMLSpace<UChar>)

[tkent, 2017/04/25 09:56:34]

The argument of textContent() means convert_brs_to_newlines, and the
specification doesn't say we should convert BRs. So the argument should be
|false|.

[yuzuchan, 2017/04/26 08:54:57]

Right, done.

+                                .SimplifyWhiteSpace(IsHTMLSpace<UChar>);
+}
+
+void HTMLMenuItemElement::setLabel(const AtomicString& label) {
+  setAttribute(labelAttr, label);
+}
+
 DEFINE_NODE_FACTORY(HTMLMenuItemElement)
 
 }  // namespace blink