From 47209070afd4d14f399cc3a9f12f6c2394d0e579 Mon Sep 17 00:00:00 2001 From: Jessica Jong Date: Thu, 3 Aug 2017 01:27:00 -0400 Subject: [PATCH] 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 | 3 +++ dom/html/HTMLInputElement.cpp | 24 +++++++++++++++------ dom/html/HTMLInputElement.h | 17 ++++++++------- dom/html/HTMLSelectElement.cpp | 11 +++++----- dom/html/HTMLSelectElement.h | 2 +- dom/html/HTMLTextAreaElement.cpp | 13 ++++++------ dom/html/HTMLTextAreaElement.h | 3 ++- dom/html/nsGenericHTMLElement.cpp | 35 +++++++++++++++++++++++++++++++ dom/html/nsGenericHTMLElement.h | 5 +++++ 9 files changed, 86 insertions(+), 27 deletions(-) diff --git a/dom/events/EventStates.h b/dom/events/EventStates.h index 8e3486b8fe3b..eba3197fa285 100644 --- a/dom/events/EventStates.h +++ b/dom/events/EventStates.h @@ -330,6 +330,8 @@ private: #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. // @@ -350,6 +352,7 @@ private: MANUALLY_MANAGED_STATES | \ DIR_ATTR_STATES | \ DISABLED_STATES | \ + REQUIRED_STATES | \ NS_EVENT_STATE_ACTIVE | \ NS_EVENT_STATE_DRAGOVER | \ NS_EVENT_STATE_FOCUS | \ diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 4672be27e270..1112a926e57c 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -1453,6 +1453,13 @@ HTMLInputElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, 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. @@ -5005,6 +5012,17 @@ HTMLInputElement::HandleTypeChange(uint8_t aNewType, bool aNotify) 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. @@ -6554,12 +6572,6 @@ HTMLInputElement::IntrinsicState() const 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; diff --git a/dom/html/HTMLInputElement.h b/dom/html/HTMLInputElement.h index 4b108c53f0f7..79b5bf94e7db 100644 --- a/dom/html/HTMLInputElement.h +++ b/dom/html/HTMLInputElement.h @@ -942,11 +942,19 @@ public: 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: @@ -1145,11 +1153,6 @@ protected: */ 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. */ diff --git a/dom/html/HTMLSelectElement.cpp b/dom/html/HTMLSelectElement.cpp index b4576e622da5..1076d3afc3ef 100644 --- a/dom/html/HTMLSelectElement.cpp +++ b/dom/html/HTMLSelectElement.cpp @@ -1333,6 +1333,11 @@ HTMLSelectElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, 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. @@ -1530,12 +1535,6 @@ HTMLSelectElement::IntrinsicState() const } } - if (Required()) { - state |= NS_EVENT_STATE_REQUIRED; - } else { - state |= NS_EVENT_STATE_OPTIONAL; - } - return state; } diff --git a/dom/html/HTMLSelectElement.h b/dom/html/HTMLSelectElement.h index d40af5717d00..f6454338cc94 100644 --- a/dom/html/HTMLSelectElement.h +++ b/dom/html/HTMLSelectElement.h @@ -211,7 +211,7 @@ public: } bool Required() const { - return GetBoolAttr(nsGkAtoms::required); + return State().HasState(NS_EVENT_STATE_REQUIRED); } void SetRequired(bool aVal, ErrorResult& aRv) { diff --git a/dom/html/HTMLTextAreaElement.cpp b/dom/html/HTMLTextAreaElement.cpp index 79390fc89e46..2ccb1567c18b 100644 --- a/dom/html/HTMLTextAreaElement.cpp +++ b/dom/html/HTMLTextAreaElement.cpp @@ -958,12 +958,6 @@ 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; @@ -1114,6 +1108,13 @@ HTMLTextAreaElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, 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. diff --git a/dom/html/HTMLTextAreaElement.h b/dom/html/HTMLTextAreaElement.h index 882d8231b113..1dbcec99b431 100644 --- a/dom/html/HTMLTextAreaElement.h +++ b/dom/html/HTMLTextAreaElement.h @@ -254,7 +254,7 @@ public: } bool Required() const { - return GetBoolAttr(nsGkAtoms::required); + return State().HasState(NS_EVENT_STATE_REQUIRED); } void SetRangeText(const nsAString& aReplacement, ErrorResult& aRv); @@ -410,6 +410,7 @@ protected: void GetSelectionRange(uint32_t* aSelectionStart, uint32_t* aSelectionEnd, ErrorResult& aRv); + private: static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes, GenericSpecifiedValues* aGenericData); diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index bf9e4bb4c32e..75351c8fe260 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -109,6 +109,7 @@ #include "mozilla/StyleSetHandleInlines.h" #include "ReferrerPolicy.h" #include "mozilla/dom/HTMLLabelElement.h" +#include "mozilla/dom/HTMLInputElement.h" using namespace mozilla; using namespace mozilla::dom; @@ -2425,6 +2426,40 @@ void nsGenericHTMLFormElement::UpdateDisabledState(bool 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) { diff --git a/dom/html/nsGenericHTMLElement.h b/dom/html/nsGenericHTMLElement.h index f41ff53bee49..5c8be551c6d2 100644 --- a/dom/html/nsGenericHTMLElement.h +++ b/dom/html/nsGenericHTMLElement.h @@ -1101,6 +1101,11 @@ public: */ 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); }