Bug 1365399 - Handle selectedness and dirtiness correctly in Option constructor. r=smaug

This bug fixes this statement in the spec (https://html.spec.whatwg.org/#dom-option):
  If selected is true, then set option's selectedness to true; otherwise set its
  selectedness to false (even if defaultSelected is true).
And also reset the dirtiness.

This patch also reverts the changes in Bug 927796 and 942648, which was added
because IsSelected() returned inaccurately (called from SelectSomething()).
Verified that the tests added in Bug 927796 and 942648 do pass.

MozReview-Commit-ID: 8PVgvCIHPj4

dom/html/HTMLOptionElement.cpp

@@ -151,11 +151,6 @@ HTMLOptionElement::Index()
 bool
 HTMLOptionElement::Selected() const
 {
-  // If we haven't been explictly selected or deselected, use our default value
-  if (!mSelectedChanged) {
-    return DefaultSelected();
-  }
-
   return mIsSelected;
 }
 
@@ -193,21 +188,14 @@ HTMLOptionElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
     return NS_OK;
   }
 
-  bool defaultSelected = aValue;
-  // First make sure we actually set our mIsSelected state to reflect our new
-  // defaultSelected state.  If that turns out to be wrong,
-  // SetOptionsSelectedByIndex will fix it up.  But otherwise we can end up in a
-  // situation where mIsSelected is still false, but mSelectedChanged becomes
-  // true (later in this method, when we compare mIsSelected to
-  // defaultSelected), and then we start returning false for Selected() even
-  // though we're actually selected.
-  mIsSelected = defaultSelected;
-
   // We just changed out selected state (since we look at the "selected"
   // attribute when mSelectedChanged is false).  Let's tell our select about
   // it.
   HTMLSelectElement* selectInt = GetSelect();
   if (!selectInt) {
+    // If option is a child of select, SetOptionsSelectedByIndex will set
+    // mIsSelected if needed.
+    mIsSelected = aValue;
     return NS_OK;
   }
 
@@ -218,7 +206,7 @@ HTMLOptionElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
 
   int32_t index = Index();
   uint32_t mask = HTMLSelectElement::SET_DISABLED;
-  if (defaultSelected) {
+  if (aValue) {
     mask |= HTMLSelectElement::IS_SELECTED;
   }
 
@@ -236,8 +224,8 @@ HTMLOptionElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName,
   // rigt selected state.
   mIsInSetDefaultSelected = inSetDefaultSelected;
   // mIsSelected might have been changed by SetOptionsSelectedByIndex.  Possibly
-  // more than once; make sure our mSelectedChanged state is set correctly.
-  mSelectedChanged = mIsSelected != defaultSelected;
+  // more than once; make sure our mSelectedChanged state is set back correctly.
+  mSelectedChanged = false;
 
   return NS_OK;
 }
@@ -426,13 +414,13 @@ HTMLOptionElement::Option(const GlobalObject& aGlobal,
     }
   }
 
-  if (aSelected) {
-    option->SetSelected(true, aError);
-    if (aError.Failed()) {
-      return nullptr;
-    }
+  option->SetSelected(aSelected, aError);
+  if (aError.Failed()) {
+    return nullptr;
   }
 
+  option->SetSelectedChanged(false);
+
   return option.forget();
 }
 

dom/html/test/test_bug596511.html

@@ -186,7 +186,7 @@ function checkInvalidWhenValueMissing(element)
 
   // Setting defaultSelected to true should not make the option selected
   select.add(new Option("", "", true), null);
-  checkSufferingFromBeingMissing(select, true);
+  checkSufferingFromBeingMissing(select);
   select.remove(0);
 
   select.add(new Option("", "", true, true), null);
@@ -210,13 +210,13 @@ function checkInvalidWhenValueMissing(element)
   checkSufferingFromBeingMissing(select);
 
   select.add(new Option("", "", true), null);
-  checkSufferingFromBeingMissing(select, true);
+  checkSufferingFromBeingMissing(select);
 
   select.add(new Option("", "", true), null);
-  checkSufferingFromBeingMissing(select, true);
+  checkSufferingFromBeingMissing(select);
 
   select.add(new Option("foo"), null);
-  checkSufferingFromBeingMissing(select, true);
+  checkSufferingFromBeingMissing(select);
 
   select.options[2].selected = true;
   checkNotSufferingFromBeingMissing(select);

testing/web-platform/meta/html/semantics/forms/the-option-element/option-element-constructor.html.ini

@@ -1,11 +0,0 @@
-[option-element-constructor.html]
-  type: testharness
-  [Option constructor handles selectedness correctly when specified with defaultSelected only]
-    expected: FAIL
-
-  [Option constructor does not set dirtiness (so, manipulating the selected content attribute still updates the selected IDL attribute)]
-    expected: FAIL
-
-  [Option constructor handles selectedness correctly, even when incongruous with defaultSelected]
-    expected: FAIL
-