Bug 1385478 - Part 2: Change Required/IsRequired() to look at NS_EVENT_STATE_REQUIRED instead. r=bz
authorJessica Jong <jjong@mozilla.com>
Thu, 03 Aug 2017 01:27:00 -0400
changeset 372654 2aa4fe1bb445
parent 372653 00412d7d8f38
child 372655 e9e0aeeda27d
push id93357
push userryanvm@gmail.com
push dateThu, 03 Aug 2017 13:11:42 +0000
treeherdermozilla-inbound@2aa4fe1bb445 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1385478
milestone57.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 1385478 - Part 2: Change Required/IsRequired() to look at NS_EVENT_STATE_REQUIRED instead. r=bz In order to speed up Required/IsRequired(), instead of querying for the @required attribute, we're now using the NS_EVENT_STATE_REQUIRED flag to know whether an element's value is required. For this to work correctly, we need to set NS_EVENT_STATE_REQUIRED earlier, that is, in AfterSetAttr(), before any consumer of IsRequired(). We also need to update or clear our required states when input type changes, since we may have changed from a required input type to a non-required input type or vice versa. Note that NS_EVENT_STATE_REQUIRED/OPTIONAL is now part of the EXTERNALLY_MANAGED_STATES. MozReview-Commit-ID: Bjiby9GqJSB
dom/events/EventStates.h
dom/html/HTMLInputElement.cpp
dom/html/HTMLInputElement.h
dom/html/HTMLSelectElement.cpp
dom/html/HTMLSelectElement.h
dom/html/HTMLTextAreaElement.cpp
dom/html/HTMLTextAreaElement.h
dom/html/nsGenericHTMLElement.cpp
dom/html/nsGenericHTMLElement.h
--- a/dom/events/EventStates.h
+++ b/dom/events/EventStates.h
@@ -325,16 +325,18 @@ private:
 
 #define DIR_ATTR_STATES (NS_EVENT_STATE_HAS_DIR_ATTR |          \
                          NS_EVENT_STATE_DIR_ATTR_LTR |          \
                          NS_EVENT_STATE_DIR_ATTR_RTL |          \
                          NS_EVENT_STATE_DIR_ATTR_LIKE_AUTO)
 
 #define DISABLED_STATES (NS_EVENT_STATE_DISABLED | NS_EVENT_STATE_ENABLED)
 
+#define REQUIRED_STATES (NS_EVENT_STATE_REQUIRED | NS_EVENT_STATE_OPTIONAL)
+
 // Event states that can be added and removed through
 // Element::{Add,Remove}ManuallyManagedStates.
 //
 // Take care when manually managing state bits.  You are responsible for
 // setting or clearing the bit when an Element is added or removed from a
 // document (e.g. in BindToTree and UnbindFromTree), if that is an
 // appropriate thing to do for your state bit.
 #define MANUALLY_MANAGED_STATES (             \
@@ -345,16 +347,17 @@ private:
 // Event states that are managed externally to an element (by the
 // EventStateManager, or by other code).  As opposed to those in
 // INTRINSIC_STATES, which are are computed by the element itself
 // and returned from Element::IntrinsicState.
 #define EXTERNALLY_MANAGED_STATES (           \
   MANUALLY_MANAGED_STATES |                   \
   DIR_ATTR_STATES |                           \
   DISABLED_STATES |                           \
+  REQUIRED_STATES |                           \
   NS_EVENT_STATE_ACTIVE |                     \
   NS_EVENT_STATE_DRAGOVER |                   \
   NS_EVENT_STATE_FOCUS |                      \
   NS_EVENT_STATE_FOCUSRING |                  \
   NS_EVENT_STATE_FOCUS_WITHIN |               \
   NS_EVENT_STATE_FULL_SCREEN |                \
   NS_EVENT_STATE_HOVER |                      \
   NS_EVENT_STATE_UNRESOLVED |                 \
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@@ -1448,16 +1448,23 @@ HTMLInputElement::AfterSetAttr(int32_t a
         aName == nsGkAtoms::readonly) {
       if (aName == nsGkAtoms::disabled) {
         // This *has* to be called *before* validity state check because
         // UpdateBarredFromConstraintValidation and
         // UpdateValueMissingValidityState depend on our disabled state.
         UpdateDisabledState(aNotify);
       }
 
+      if (aName == nsGkAtoms::required && DoesRequiredApply()) {
+        // This *has* to be called *before* UpdateValueMissingValidityState
+        // because UpdateValueMissingValidityState depends on our required
+        // state.
+        UpdateRequiredState(!!aValue, aNotify);
+      }
+
       UpdateValueMissingValidityState();
 
       // This *has* to be called *after* validity has changed.
       if (aName == nsGkAtoms::readonly || aName == nsGkAtoms::disabled) {
         UpdateBarredFromConstraintValidation();
       }
     } else if (aName == nsGkAtoms::maxlength) {
       UpdateTooLongValidityState();
@@ -5000,16 +5007,27 @@ HTMLInputElement::HandleTypeChange(uint8
   // previous type does, we should clear out mFocusedValue.
   if (MayFireChangeOnBlur(mType) && !MayFireChangeOnBlur(oldType)) {
     GetValue(mFocusedValue, CallerType::System);
   } else if (!IsSingleLineTextControl(false, mType) &&
              IsSingleLineTextControl(false, oldType)) {
     mFocusedValue.Truncate();
   }
 
+  // Update or clear our required states since we may have changed from a
+  // required input type to a non-required input type or viceversa.
+  if (DoesRequiredApply()) {
+    bool isRequired = HasAttr(kNameSpaceID_None, nsGkAtoms::required);
+    UpdateRequiredState(isRequired, aNotify);
+  } else if (aNotify) {
+    RemoveStates(REQUIRED_STATES);
+  } else {
+    RemoveStatesSilently(REQUIRED_STATES);
+  }
+
   UpdateHasRange();
 
   // Do not notify, it will be done after if needed.
   UpdateAllValidityStates(false);
 
   UpdateApzAwareFlag();
 
   UpdateBarredFromConstraintValidation();
@@ -6549,22 +6567,16 @@ HTMLInputElement::IntrinsicState() const
     // Check whether we are the default checked element (:default)
     if (DefaultChecked()) {
       state |= NS_EVENT_STATE_DEFAULT;
     }
   } else if (mType == NS_FORM_INPUT_IMAGE) {
     state |= nsImageLoadingContent::ImageState();
   }
 
-  if (DoesRequiredApply() && IsRequired()) {
-    state |= NS_EVENT_STATE_REQUIRED;
-  } else {
-    state |= NS_EVENT_STATE_OPTIONAL;
-  }
-
   if (IsCandidateForConstraintValidation()) {
     if (IsValid()) {
       state |= NS_EVENT_STATE_VALID;
     } else {
       state |= NS_EVENT_STATE_INVALID;
 
       if ((!mForm || !mForm->HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate)) &&
           (GetValidityState(VALIDITY_STATE_CUSTOM_ERROR) ||
--- a/dom/html/HTMLInputElement.h
+++ b/dom/html/HTMLInputElement.h
@@ -937,21 +937,29 @@ public:
    */
   static Decimal StringToDecimal(const nsAString& aValue);
 
   void UpdateEntries(const nsTArray<OwningFileOrDirectory>& aFilesOrDirectories);
 
   static void Shutdown();
 
   /**
-   * Returns the current required state of the element.
+   * Returns if the required attribute applies for the current type.
+   */
+  bool DoesRequiredApply() const;
+
+  /**
+   * Returns the current required state of the element. This function differs
+   * from Required() in that this function only returns true for input types
+   * that @required attribute applies and the attribute is set; in contrast,
+   * Required() returns true whenever @required attribute is set.
    */
   bool IsRequired() const
   {
-    return HasAttr(kNameSpaceID_None, nsGkAtoms::required);
+    return State().HasState(NS_EVENT_STATE_REQUIRED);
   }
 
 protected:
   virtual ~HTMLInputElement();
 
   virtual JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   // Pull IsSingleLineTextControl into our scope, otherwise it'd be hidden
@@ -1141,21 +1149,16 @@ protected:
   bool IsMutable() const;
 
   /**
    * Returns if the readonly attribute applies for the current type.
    */
   bool DoesReadOnlyApply() const;
 
   /**
-   * Returns if the required attribute applies for the current type.
-   */
-  bool DoesRequiredApply() const;
-
-  /**
    * Returns if the min and max attributes apply for the current type.
    */
   bool DoesMinMaxApply() const;
 
   /**
    * Returns if the step attribute apply for the current type.
    */
   bool DoesStepApply() const { return DoesMinMaxApply(); }
--- a/dom/html/HTMLSelectElement.cpp
+++ b/dom/html/HTMLSelectElement.cpp
@@ -1328,16 +1328,21 @@ HTMLSelectElement::AfterSetAttr(int32_t 
       // This *has* to be called *before* validity state check because
       // UpdateBarredFromConstraintValidation and
       // UpdateValueMissingValidityState depend on our disabled state.
       UpdateDisabledState(aNotify);
 
       UpdateValueMissingValidityState();
       UpdateBarredFromConstraintValidation();
     } else if (aName == nsGkAtoms::required) {
+      // This *has* to be called *before* UpdateValueMissingValidityState
+      // because UpdateValueMissingValidityState depends on our required
+      // state.
+      UpdateRequiredState(!!aValue, aNotify);
+
       UpdateValueMissingValidityState();
     } else if (aName == nsGkAtoms::autocomplete) {
       // Clear the cached @autocomplete attribute and autocompleteInfo state.
       mAutocompleteAttrState = nsContentUtils::eAutocompleteAttrState_Unknown;
       mAutocompleteInfoState = nsContentUtils::eAutocompleteAttrState_Unknown;
     } else if (aName == nsGkAtoms::multiple) {
       if (!aValue && aNotify) {
         // We might have become a combobox; make sure _something_ gets
@@ -1525,22 +1530,16 @@ HTMLSelectElement::IntrinsicState() cons
     if ((!mForm || !mForm->HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate)) &&
         (mCanShowValidUI && ShouldShowValidityUI() &&
          (IsValid() || (state.HasState(NS_EVENT_STATE_MOZ_UI_INVALID) &&
                         !mCanShowInvalidUI)))) {
       state |= NS_EVENT_STATE_MOZ_UI_VALID;
     }
   }
 
-  if (Required()) {
-    state |= NS_EVENT_STATE_REQUIRED;
-  } else {
-    state |= NS_EVENT_STATE_OPTIONAL;
-  }
-
   return state;
 }
 
 // nsIFormControl
 
 NS_IMETHODIMP
 HTMLSelectElement::SaveState()
 {
--- a/dom/html/HTMLSelectElement.h
+++ b/dom/html/HTMLSelectElement.h
@@ -206,17 +206,17 @@ public:
   }
   // Uses XPCOM GetName.
   void SetName(const nsAString& aName, ErrorResult& aRv)
   {
     SetHTMLAttr(nsGkAtoms::name, aName, aRv);
   }
   bool Required() const
   {
-    return GetBoolAttr(nsGkAtoms::required);
+    return State().HasState(NS_EVENT_STATE_REQUIRED);
   }
   void SetRequired(bool aVal, ErrorResult& aRv)
   {
     SetHTMLBoolAttr(nsGkAtoms::required, aVal, aRv);
   }
   uint32_t Size() const
   {
     return GetUnsignedIntAttr(nsGkAtoms::size, 0);
--- a/dom/html/HTMLTextAreaElement.cpp
+++ b/dom/html/HTMLTextAreaElement.cpp
@@ -953,22 +953,16 @@ HTMLTextAreaElement::RestoreState(nsPres
   return false;
 }
 
 EventStates
 HTMLTextAreaElement::IntrinsicState() const
 {
   EventStates state = nsGenericHTMLFormElementWithState::IntrinsicState();
 
-  if (Required()) {
-    state |= NS_EVENT_STATE_REQUIRED;
-  } else {
-    state |= NS_EVENT_STATE_OPTIONAL;
-  }
-
   if (IsCandidateForConstraintValidation()) {
     if (IsValid()) {
       state |= NS_EVENT_STATE_VALID;
     } else {
       state |= NS_EVENT_STATE_INVALID;
       // :-moz-ui-invalid always apply if the element suffers from a custom
       // error and never applies if novalidate is set on the form owner.
       if ((!mForm || !mForm->HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate)) &&
@@ -1109,16 +1103,23 @@ HTMLTextAreaElement::AfterSetAttr(int32_
         aName == nsGkAtoms::readonly) {
       if (aName == nsGkAtoms::disabled) {
         // This *has* to be called *before* validity state check because
         // UpdateBarredFromConstraintValidation and
         // UpdateValueMissingValidityState depend on our disabled state.
         UpdateDisabledState(aNotify);
       }
 
+      if (aName == nsGkAtoms::required) {
+        // This *has* to be called *before* UpdateValueMissingValidityState
+        // because UpdateValueMissingValidityState depends on our required
+        // state.
+        UpdateRequiredState(!!aValue, aNotify);
+      }
+
       UpdateValueMissingValidityState();
 
       // This *has* to be called *after* validity has changed.
       if (aName == nsGkAtoms::readonly || aName == nsGkAtoms::disabled) {
         UpdateBarredFromConstraintValidation();
       }
     } else if (aName == nsGkAtoms::maxlength) {
       UpdateTooLongValidityState();
--- a/dom/html/HTMLTextAreaElement.h
+++ b/dom/html/HTMLTextAreaElement.h
@@ -249,17 +249,17 @@ public:
     return GetBoolAttr(nsGkAtoms::readonly);
   }
   void SetReadOnly(bool aReadOnly, ErrorResult& aError)
   {
     SetHTMLBoolAttr(nsGkAtoms::readonly, aReadOnly, aError);
   }
   bool Required() const
   {
-    return GetBoolAttr(nsGkAtoms::required);
+    return State().HasState(NS_EVENT_STATE_REQUIRED);
   }
 
   void SetRangeText(const nsAString& aReplacement, ErrorResult& aRv);
 
   void SetRangeText(const nsAString& aReplacement, uint32_t aStart,
                     uint32_t aEnd, SelectionMode aSelectMode,
                     ErrorResult& aRv);
 
@@ -405,16 +405,17 @@ protected:
 
   /**
    * A helper to get the current selection range.  Will throw on the ErrorResult
    * if we have no editor state.
    */
   void GetSelectionRange(uint32_t* aSelectionStart,
                          uint32_t* aSelectionEnd,
                          ErrorResult& aRv);
+
 private:
   static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
                                     GenericSpecifiedValues* aGenericData);
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -104,16 +104,17 @@
 #include "nsGlobalWindow.h"
 #include "mozilla/dom/HTMLBodyElement.h"
 #include "imgIContainer.h"
 #include "nsComputedDOMStyle.h"
 #include "mozilla/StyleSetHandle.h"
 #include "mozilla/StyleSetHandleInlines.h"
 #include "ReferrerPolicy.h"
 #include "mozilla/dom/HTMLLabelElement.h"
+#include "mozilla/dom/HTMLInputElement.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
 /**
  * nsAutoFocusEvent is used to dispatch a focus event when a
  * nsGenericHTMLFormElement is binded to the tree with the autofocus attribute
  * enabled.
@@ -2421,16 +2422,50 @@ void nsGenericHTMLFormElement::UpdateDis
   EventStates changedStates = disabledStates ^ oldDisabledStates;
 
   if (!changedStates.IsEmpty()) {
     ToggleStates(changedStates, aNotify);
   }
 }
 
 void
+nsGenericHTMLFormElement::UpdateRequiredState(bool aIsRequired, bool aNotify)
+{
+#ifdef DEBUG
+  int32_t type = ControlType();
+#endif
+  MOZ_ASSERT((type & NS_FORM_INPUT_ELEMENT) ||
+              type == NS_FORM_SELECT ||
+              type == NS_FORM_TEXTAREA,
+             "This should be called only on types that @required applies");
+
+#ifdef DEBUG
+  HTMLInputElement* input = HTMLInputElement::FromContent(this);
+  if (input) {
+    MOZ_ASSERT(input->DoesRequiredApply(),
+               "This should be called only on input types that @required applies");
+  }
+#endif
+
+  EventStates requiredStates;
+  if (aIsRequired) {
+    requiredStates |= NS_EVENT_STATE_REQUIRED;
+  } else {
+    requiredStates |= NS_EVENT_STATE_OPTIONAL;
+  }
+
+  EventStates oldRequiredStates = State() & REQUIRED_STATES;
+  EventStates changedStates = requiredStates ^ oldRequiredStates;
+
+  if (!changedStates.IsEmpty()) {
+    ToggleStates(changedStates, aNotify);
+  }
+}
+
+void
 nsGenericHTMLFormElement::FieldSetDisabledChanged(bool aNotify)
 {
   UpdateDisabledState(aNotify);
 }
 
 bool
 nsGenericHTMLFormElement::IsLabelable() const
 {
--- a/dom/html/nsGenericHTMLElement.h
+++ b/dom/html/nsGenericHTMLElement.h
@@ -1096,16 +1096,21 @@ public:
   virtual void FieldSetDisabledChanged(bool aNotify);
 
   /**
    * Check our disabled content attribute and fieldset's (if it exists) disabled
    * state to decide whether our disabled flag should be toggled.
    */
   void UpdateDisabledState(bool aNotify);
 
+  /**
+   * Update our required/optional flags to match the given aIsRequired boolean.
+   */
+  void UpdateRequiredState(bool aIsRequired, bool aNotify);
+
   void FieldSetFirstLegendChanged(bool aNotify) {
     UpdateFieldSet(aNotify);
   }
 
   /**
    * This callback is called by a fieldset on all it's elements when it's being
    * destroyed. When called, the elements should check that aFieldset is there
    * first parent fieldset and null mFieldset in that case only.