Implement HTMLMenuItemElement.label

third_party/WebKit/Source/core/html/HTMLMenuItemElement.cpp

Index: third_party/WebKit/Source/core/html/HTMLMenuItemElement.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLMenuItemElement.cpp b/third_party/WebKit/Source/core/html/HTMLMenuItemElement.cpp
index 20502c4aa363ecfaa2afd8ae0a63951b059b6cc8..9c9e149586b6471d14e0993f51eb9e87eedce774 100644
--- a/third_party/WebKit/Source/core/html/HTMLMenuItemElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMenuItemElement.cpp
@@ -4,6 +4,7 @@
 
 #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"
@@ -57,6 +58,18 @@ void HTMLMenuItemElement::DefaultEventHandler(Event* event) {
   }
 }
 
+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