Bug 677463 - <menuitem>.label should return .textContent if there's no label content attribute r=smaug
authorJan Varga <jan.varga@gmail.com>
Thu, 11 Aug 2011 08:07:26 +0200
changeset 74202 7871abb0e291ea276fc701bcd7cd22854c83b63e
parent 74201 aa9f527a4928ed675a09eb28d30b0ed63a857575
child 74203 86abf721b3fee31348afa11cf81aa5416151e4ec
push id20964
push userJan.Varga@gmail.com
push dateThu, 11 Aug 2011 06:26:35 +0000
treeherdermozilla-central@7871abb0e291 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs677463
milestone8.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 677463 - <menuitem>.label should return .textContent if there's no label content attribute r=smaug
browser/base/content/test/subtst_contextmenu.html
browser/base/content/test/test_contextmenu.html
content/html/content/src/nsGenericHTMLElement.h
content/html/content/src/nsHTMLMenuItemElement.cpp
content/html/content/src/nsHTMLMenuItemElement.h
content/html/content/src/nsHTMLOptionElement.cpp
content/html/content/test/Makefile.in
content/html/content/test/test_bug677463.html
--- a/browser/base/content/test/subtst_contextmenu.html
+++ b/browser/base/content/test/subtst_contextmenu.html
@@ -21,16 +21,17 @@ Browser context menu subtest.
 <textarea id="test-textarea">chssseesbbbie</textarea> <!-- a weird word which generates only one suggestion -->
 <div id="test-contenteditable" contenteditable="true">chssseefsbbbie</div> <!-- a more weird word which generates no suggestions -->
 <input id="test-input-spellcheck" type="text" spellcheck="true" autofocus value="prodkjfgigrty"> <!-- this one also generates one suggestion -->
 <div contextmenu="myMenu">
   <p id="test-pagemenu" hopeless="true">I've got a context menu!</p>
   <menu id="myMenu" type="context">
     <menuitem label="Plain item" onclick="document.getElementById('test-pagemenu').removeAttribute('hopeless');"></menuitem>
     <menuitem label="Disabled item" disabled></menuitem>
+    <menuitem> Item w/ textContent</menuitem>
     <menu>
       <menuitem type="checkbox" label="Checkbox" checked></menuitem>
     </menu>
     <menu>
       <menuitem type="radio" label="Radio1" checked></menuitem>
       <menuitem type="radio" label="Radio2"></menuitem>
       <menuitem type="radio" label="Radio3"></menuitem>
     </menu>
--- a/browser/base/content/test/test_contextmenu.html
+++ b/browser/base/content/test/test_contextmenu.html
@@ -471,16 +471,17 @@ function runTest(testNum) {
         closeContextMenu();
         openContextMenuFor(pagemenu); // Invoke context menu for next test.
         break;
 
     case 16:
         // Context menu for element with assigned content context menu
         checkContextMenu(["+Plain item",          {type: "", icon: "", checked: false, disabled: false},
                           "+Disabled item",       {type: "", icon: "", checked: false, disabled: true},
+                          "+Item w/ textContent", {type: "", icon: "", checked: false, disabled: false},
                           "---",                  null,
                           "+Checkbox",            {type: "checkbox", icon: "", checked: true, disabled: false},
                           "---",                  null,
                           "+Radio1",              {type: "checkbox", icon: "", checked: true, disabled: false},
                           "+Radio2",              {type: "checkbox", icon: "", checked: false, disabled: false},
                           "+Radio3",              {type: "checkbox", icon: "", checked: false, disabled: false},
                           "---",                  null,
                           "+Item w/ icon",        {type: "", icon: "favicon.ico", checked: false, disabled: false},
--- a/content/html/content/src/nsGenericHTMLElement.h
+++ b/content/html/content/src/nsGenericHTMLElement.h
@@ -1099,16 +1099,35 @@ protected:
   }                                                                  \
   NS_IMETHODIMP                                                      \
   _class::Set##_method(const nsAString& aValue)                      \
   {                                                                  \
     return SetAttrHelper(nsGkAtoms::_atom, aValue);                  \
   }
 
 /**
+ * This macro is similar to NS_IMPL_STRING_ATTR except that the getter method
+ * falls back to an alternative method if the content attribute isn't set.
+ */
+#define NS_IMPL_STRING_ATTR_WITH_FALLBACK(_class, _method, _atom, _fallback) \
+  NS_IMETHODIMP                                                              \
+  _class::Get##_method(nsAString& aValue)                                    \
+  {                                                                          \
+    if (!GetAttr(kNameSpaceID_None, nsGkAtoms::_atom, aValue)) {             \
+      _fallback(aValue);                                                     \
+    }                                                                        \
+    return NS_OK;                                                            \
+  }                                                                          \
+  NS_IMETHODIMP                                                              \
+  _class::Set##_method(const nsAString& aValue)                              \
+  {                                                                          \
+    return SetAttrHelper(nsGkAtoms::_atom, aValue);                          \
+  }
+
+/**
  * A macro to implement the getter and setter for a given boolean
  * valued content property. The method uses the generic GetAttr and
  * SetAttr methods.
  */
 #define NS_IMPL_BOOL_ATTR(_class, _method, _atom)                     \
   NS_IMETHODIMP                                                       \
   _class::Get##_method(PRBool* aValue)                                \
   {                                                                   \
--- a/content/html/content/src/nsHTMLMenuItemElement.cpp
+++ b/content/html/content/src/nsHTMLMenuItemElement.cpp
@@ -243,17 +243,18 @@ nsHTMLMenuItemElement::Clone(nsINodeInfo
   }
 
   return rv;
 }
 
 
 NS_IMPL_ENUM_ATTR_DEFAULT_VALUE(nsHTMLMenuItemElement, Type, type,
                                 kMenuItemDefaultType->tag)
-NS_IMPL_STRING_ATTR(nsHTMLMenuItemElement, Label, label)
+// GetText returns a whitespace compressed .textContent value.
+NS_IMPL_STRING_ATTR_WITH_FALLBACK(nsHTMLMenuItemElement, Label, label, GetText)
 NS_IMPL_URI_ATTR(nsHTMLMenuItemElement, Icon, icon)
 NS_IMPL_BOOL_ATTR(nsHTMLMenuItemElement, Disabled, disabled)
 NS_IMPL_BOOL_ATTR(nsHTMLMenuItemElement, DefaultChecked, checked)
 //NS_IMPL_BOOL_ATTR(nsHTMLMenuItemElement, Checked, checked)
 NS_IMPL_STRING_ATTR(nsHTMLMenuItemElement, Radiogroup, radiogroup)
 
 NS_IMETHODIMP
 nsHTMLMenuItemElement::GetChecked(PRBool* aChecked)
@@ -403,16 +404,26 @@ nsHTMLMenuItemElement::DoneCreatingEleme
   mParserCreating = false;
 
   if (mShouldInitChecked) {
     InitChecked();
     mShouldInitChecked = false;
   }
 }
 
+void
+nsHTMLMenuItemElement::GetText(nsAString& aText)
+{
+  nsAutoString text;
+  nsContentUtils::GetNodeTextContent(this, PR_FALSE, text);
+
+  text.CompressWhitespace(PR_TRUE, PR_TRUE);
+  aText = text;
+}
+
 nsresult
 nsHTMLMenuItemElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                                     const nsAString* aValue, PRBool aNotify)
 {
   if (aNameSpaceID == kNameSpaceID_None) {
     if ((aName == nsGkAtoms::radiogroup || aName == nsGkAtoms::type) &&
         mType == CMD_TYPE_RADIO &&
         !mParserCreating) {
--- a/content/html/content/src/nsHTMLMenuItemElement.h
+++ b/content/html/content/src/nsHTMLMenuItemElement.h
@@ -95,16 +95,18 @@ public:
   PRUint8 GetType() const { return mType; }
 
   /**
    * Syntax sugar to make it easier to check for checked and checked dirty
    */
   PRBool IsChecked() const { return mChecked; }
   PRBool IsCheckedDirty() const { return mCheckedDirty; }
 
+  void GetText(nsAString& aText);
+
 protected:
   virtual nsresult AfterSetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
                                 const nsAString* aValue, PRBool aNotify);
 
   void WalkRadioGroup(Visitor* aVisitor);
 
   nsHTMLMenuItemElement* GetSelectedRadio();
 
--- a/content/html/content/src/nsHTMLOptionElement.cpp
+++ b/content/html/content/src/nsHTMLOptionElement.cpp
@@ -63,36 +63,16 @@
 #include "nsIDocument.h"
 #include "nsIDOMDocument.h"
 #include "nsContentCreatorFunctions.h"
 #include "mozAutoDocUpdate.h"
 
 using namespace mozilla::dom;
 
 /**
- * This macro is similar to NS_IMPL_STRING_ATTR except that the getter method
- * falls back to GetText if the content attribute isn't set. GetText returns a
- * whitespace compressed .textContent value.
- */
-#define NS_IMPL_STRING_ATTR_WITH_TEXTCONTENT(_class, _method, _atom) \
-  NS_IMETHODIMP                                                      \
-  _class::Get##_method(nsAString& aValue)                            \
-  {                                                                  \
-    if (!GetAttr(kNameSpaceID_None, nsGkAtoms::_atom, aValue)) {     \
-      GetText(aValue);                                               \
-    }                                                                \
-    return NS_OK;                                                    \
-  }                                                                  \
-  NS_IMETHODIMP                                                      \
-  _class::Set##_method(const nsAString& aValue)                      \
-  {                                                                  \
-    return SetAttrHelper(nsGkAtoms::_atom, aValue);                  \
-  }
-
-/**
  * Implementation of &lt;option&gt;
  */
 
 nsGenericHTMLElement*
 NS_NewHTMLOptionElement(already_AddRefed<nsINodeInfo> aNodeInfo,
                         FromParser aFromParser)
 {
   /*
@@ -204,18 +184,19 @@ nsHTMLOptionElement::SetSelected(PRBool 
     SetSelectedInternal(aValue, PR_TRUE);
     return NS_OK;
   }
 
   return NS_OK;
 }
 
 NS_IMPL_BOOL_ATTR(nsHTMLOptionElement, DefaultSelected, selected)
-NS_IMPL_STRING_ATTR_WITH_TEXTCONTENT(nsHTMLOptionElement, Label, label)
-NS_IMPL_STRING_ATTR_WITH_TEXTCONTENT(nsHTMLOptionElement, Value, value)
+// GetText returns a whitespace compressed .textContent value.
+NS_IMPL_STRING_ATTR_WITH_FALLBACK(nsHTMLOptionElement, Label, label, GetText)
+NS_IMPL_STRING_ATTR_WITH_FALLBACK(nsHTMLOptionElement, Value, value, GetText)
 NS_IMPL_BOOL_ATTR(nsHTMLOptionElement, Disabled, disabled)
 
 NS_IMETHODIMP 
 nsHTMLOptionElement::GetIndex(PRInt32* aIndex)
 {
   NS_ENSURE_ARG_POINTER(aIndex);
 
   *aIndex = -1; // -1 indicates the index was not found
--- a/content/html/content/test/Makefile.in
+++ b/content/html/content/test/Makefile.in
@@ -277,12 +277,13 @@ include $(topsrcdir)/config/rules.mk
 		test_bug666200.html \
 		test_bug666666.html \
 		test_bug674558.html \
 		test_bug583533.html \
 		test_restore_from_parser_fragment.html \
 		test_bug617528.html \
 		test_checked.html \
 		test_bug677658.html \
+		test_bug677463.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/test_bug677463.html
@@ -0,0 +1,39 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=677463
+-->
+<head>
+  <title>Test for Bug 677463</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=677463">Mozilla Bug 677463</a>
+<p id="display"></p>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 677463 **/
+
+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");
+m.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");
+
+</script>
+</pre>
+</body>
+</html>