From 2f59167e65132fc652456ae8629c1246a9b4b2cb Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Thu, 16 Apr 2020 19:59:10 -0400 Subject: Bug 656197 - Push state updates further out across beforesetattr/aftersetattr * Remove the generic attr preparsing mechanism from BeforeSetAttr and just preparse class attributes directly in the one place that needs to do it * Move calls to BeforeSetAttr to after AttributeWillChange * Remove UpdateState calls in BeforeSetAttr * Move calls to AfterSetAttr to before UpdateState when manipulating attributes * Remove UpdateState calls from AfterSetAttr, since they are no longer needed there Tag #1375 --- dom/base/Element.cpp | 71 +++++++++++++++++---------------------- dom/base/Element.h | 11 +++--- dom/base/nsAttrValueOrString.h | 16 +++------ dom/html/HTMLButtonElement.cpp | 3 +- dom/html/HTMLButtonElement.h | 2 +- dom/html/HTMLDetailsElement.cpp | 2 +- dom/html/HTMLDetailsElement.h | 3 +- dom/html/HTMLImageElement.cpp | 2 +- dom/html/HTMLImageElement.h | 2 +- dom/html/HTMLInputElement.cpp | 4 +-- dom/html/HTMLInputElement.h | 2 +- dom/html/HTMLLinkElement.cpp | 2 +- dom/html/HTMLLinkElement.h | 2 +- dom/html/HTMLOptionElement.cpp | 2 +- dom/html/HTMLOptionElement.h | 2 +- dom/html/HTMLSelectElement.cpp | 4 +-- dom/html/HTMLSelectElement.h | 2 +- dom/html/HTMLTableElement.cpp | 2 +- dom/html/HTMLTableElement.h | 2 +- dom/html/HTMLTextAreaElement.cpp | 4 +-- dom/html/HTMLTextAreaElement.h | 2 +- dom/html/nsGenericHTMLElement.cpp | 15 +-------- dom/html/nsGenericHTMLElement.h | 2 +- dom/svg/nsSVGElement.cpp | 36 +++++++++++--------- dom/svg/nsSVGElement.h | 2 +- dom/xul/nsXULElement.cpp | 2 +- dom/xul/nsXULElement.h | 2 +- 27 files changed, 84 insertions(+), 117 deletions(-) (limited to 'dom') diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 91a3b63d7..055746885 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -2461,6 +2461,9 @@ Element::SetAttr(int32_t aNamespaceID, nsIAtom* aName, uint8_t modType; bool hasListeners; + // We don't want to spend time preparsing class attributes if the value is not + // changing, so just init our nsAttrValueOrString with aValue for the + // OnlyNotifySameValueSet call. nsAttrValueOrString value(aValue); nsAttrValue oldValue; @@ -2469,25 +2472,30 @@ Element::SetAttr(int32_t aNamespaceID, nsIAtom* aName, return NS_OK; } - nsresult rv = BeforeSetAttr(aNamespaceID, aName, &value, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - nsAttrValue* preparsedAttrValue = value.GetStoredAttrValue(); + nsAttrValue attrValue; + nsAttrValue* preparsedAttrValue; + if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::_class) { + attrValue.ParseAtomArray(aValue); + value.ResetToAttrValue(attrValue); + preparsedAttrValue = &attrValue; + } else { + preparsedAttrValue = nullptr; + } if (aNotify) { nsNodeUtils::AttributeWillChange(this, aNamespaceID, aName, modType, preparsedAttrValue); } + nsresult rv = BeforeSetAttr(aNamespaceID, aName, &value, aNotify); + NS_ENSURE_SUCCESS(rv, rv); + // Hold a script blocker while calling ParseAttribute since that can call // out to id-observers nsAutoScriptBlocker scriptBlocker; - nsAttrValue attrValue; - if (preparsedAttrValue) { - attrValue.SwapValueWith(*preparsedAttrValue); - } - // Even the value was pre-parsed in BeforeSetAttr, we still need to call - // ParseAttribute because it can have side effects. + // Even the value was pre-parsed, we still need to call ParseAttribute because + // it can have side effects. if (!ParseAttribute(aNamespaceID, aName, aValue, attrValue)) { attrValue.SetTo(aValue); } @@ -2523,14 +2531,14 @@ Element::SetParsedAttr(int32_t aNamespaceID, nsIAtom* aName, return NS_OK; } - nsresult rv = BeforeSetAttr(aNamespaceID, aName, &value, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - if (aNotify) { nsNodeUtils::AttributeWillChange(this, aNamespaceID, aName, modType, &aParsedValue); } + nsresult rv = BeforeSetAttr(aNamespaceID, aName, &value, aNotify); + NS_ENSURE_SUCCESS(rv, rv); + return SetAttrAndNotify(aNamespaceID, aName, aPrefix, oldValue, aParsedValue, modType, hasListeners, aNotify, kCallAfterSetAttr); @@ -2599,8 +2607,6 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, } } - UpdateState(aNotify); - if (CustomElementRegistry::IsCustomElementEnabled()) { if (CustomElementData* data = GetCustomElementData()) { if (CustomElementDefinition* definition = @@ -2639,6 +2645,8 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, } } + UpdateState(aNotify); + if (aNotify) { // Don't pass aOldValue to AttributeChanged since it may not be reliable. // Callers only compute aOldValue under certain conditions which may not @@ -2674,25 +2682,6 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, return NS_OK; } -nsresult -Element::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, bool aNotify) -{ - if (aNamespaceID == kNameSpaceID_None) { - if (aName == nsGkAtoms::_class) { - // aValue->GetAttrValue will only be non-null here when this is called - // via Element::SetParsedAttr. This shouldn't happen for "class", but - // this will handle it. - if (aValue && !aValue->GetAttrValue()) { - nsAttrValue attr; - attr.ParseAtomArray(aValue->String()); - aValue->TakeParsedValue(attr); - } - } - } - return NS_OK; -} - bool Element::ParseAttribute(int32_t aNamespaceID, nsIAtom* aAttribute, @@ -2809,9 +2798,6 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, return NS_OK; } - nsresult rv = BeforeSetAttr(aNameSpaceID, aName, nullptr, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - nsIDocument *document = GetComposedDoc(); mozAutoDocUpdate updateBatch(document, UPDATE_CONTENT_MODEL, aNotify); @@ -2821,6 +2807,9 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, nullptr); } + nsresult rv = BeforeSetAttr(aNameSpaceID, aName, nullptr, aNotify); + NS_ENSURE_SUCCESS(rv, rv); + bool hasMutationListeners = aNotify && nsContentUtils::HasMutationListeners(this, NS_EVENT_BITS_MUTATION_ATTRMODIFIED, @@ -2869,8 +2858,6 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, } } - UpdateState(aNotify); - if (CustomElementRegistry::IsCustomElementEnabled()) { if (CustomElementData* data = GetCustomElementData()) { if (CustomElementDefinition* definition = @@ -2897,6 +2884,11 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, } } + rv = AfterSetAttr(aNameSpaceID, aName, nullptr, aNotify); + NS_ENSURE_SUCCESS(rv, rv); + + UpdateState(aNotify); + if (aNotify) { // We can always pass oldValue here since there is no new value which could // have corrupted it. @@ -2904,9 +2896,6 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, nsIDOMMutationEvent::REMOVAL, &oldValue); } - rv = AfterSetAttr(aNameSpaceID, aName, nullptr, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::dir) { OnSetDirAttr(this, nullptr, hadValidDir, hadDirAuto, aNotify); } diff --git a/dom/base/Element.h b/dom/base/Element.h index 8139236aa..aa917bffc 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -1336,16 +1336,17 @@ protected: * @param aName the localname of the attribute being set * @param aValue the value it's being set to represented as either a string or * a parsed nsAttrValue. Alternatively, if the attr is being removed it - * will be null. BeforeSetAttr is allowed to modify aValue by parsing - * the string to an nsAttrValue (to avoid having to reparse it in - * ParseAttribute). + * will be null. * @param aNotify Whether we plan to notify document observers. */ // Note that this is inlined so that when subclasses call it it gets // inlined. Those calls don't go through a vtable. virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, - bool aNotify); + const nsAttrValueOrString* aValue, + bool aNotify) + { + return NS_OK; + } /** * Hook that is called by Element::SetAttr to allow subclasses to diff --git a/dom/base/nsAttrValueOrString.h b/dom/base/nsAttrValueOrString.h index df0dac17d..8e6c06953 100644 --- a/dom/base/nsAttrValueOrString.h +++ b/dom/base/nsAttrValueOrString.h @@ -49,20 +49,13 @@ public: , mCheapString(nullptr) { } - void TakeParsedValue(nsAttrValue& aValue) + void ResetToAttrValue(const nsAttrValue& aValue) { - mStoredAttrValue.SwapValueWith(aValue); - mAttrValue = &mStoredAttrValue; + mAttrValue = &aValue; mStringPtr = nullptr; + // No need to touch mCheapString here. If we need to use it, we will reset + // it to the rigthe value anyway. } - /** - * If TakeParsedValue has been called, returns the value that it set. - */ - nsAttrValue* GetStoredAttrValue() - { - return mAttrValue == &mStoredAttrValue ? &mStoredAttrValue : nullptr; - } - const nsAttrValue* GetAttrValue() { return mAttrValue; } /** * Returns a reference to the string value of the contents of this object. @@ -89,7 +82,6 @@ protected: const nsAttrValue* mAttrValue; mutable const nsAString* mStringPtr; mutable nsCheapString mCheapString; - nsAttrValue mStoredAttrValue; }; #endif // nsAttrValueOrString_h___ diff --git a/dom/html/HTMLButtonElement.cpp b/dom/html/HTMLButtonElement.cpp index aad1c412b..c26e60ed0 100644 --- a/dom/html/HTMLButtonElement.cpp +++ b/dom/html/HTMLButtonElement.cpp @@ -423,7 +423,7 @@ HTMLButtonElement::DoneCreatingElement() nsresult HTMLButtonElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) { if (aNotify && aName == nsGkAtoms::disabled && @@ -448,7 +448,6 @@ HTMLButtonElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, if (aName == nsGkAtoms::type || aName == nsGkAtoms::disabled) { UpdateBarredFromConstraintValidation(); - UpdateState(aNotify); } } diff --git a/dom/html/HTMLButtonElement.h b/dom/html/HTMLButtonElement.h index a8d206dd6..19c93e483 100644 --- a/dom/html/HTMLButtonElement.h +++ b/dom/html/HTMLButtonElement.h @@ -81,7 +81,7 @@ public: * Called when an attribute is about to be changed */ virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; /** * Called when an attribute has just been changed diff --git a/dom/html/HTMLDetailsElement.cpp b/dom/html/HTMLDetailsElement.cpp index 8619b1450..9d4dd89c2 100644 --- a/dom/html/HTMLDetailsElement.cpp +++ b/dom/html/HTMLDetailsElement.cpp @@ -45,7 +45,7 @@ HTMLDetailsElement::GetAttributeChangeHint(const nsIAtom* aAttribute, nsresult HTMLDetailsElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, bool aNotify) + const nsAttrValueOrString* aValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::open) { bool setOpen = aValue != nullptr; diff --git a/dom/html/HTMLDetailsElement.h b/dom/html/HTMLDetailsElement.h index 6adf567bf..4575ed888 100644 --- a/dom/html/HTMLDetailsElement.h +++ b/dom/html/HTMLDetailsElement.h @@ -38,7 +38,8 @@ public: int32_t aModType) const override; nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, bool aNotify) override; + const nsAttrValueOrString* aValue, + bool aNotify) override; // HTMLDetailsElement WebIDL bool Open() const { return GetBoolAttr(nsGkAtoms::open); } diff --git a/dom/html/HTMLImageElement.cpp b/dom/html/HTMLImageElement.cpp index 49cbebc5a..5d0cfd15e 100644 --- a/dom/html/HTMLImageElement.cpp +++ b/dom/html/HTMLImageElement.cpp @@ -369,7 +369,7 @@ HTMLImageElement::GetAttributeMappingFunction() const nsresult HTMLImageElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) { diff --git a/dom/html/HTMLImageElement.h b/dom/html/HTMLImageElement.h index 41730c2e5..d92cd84b8 100644 --- a/dom/html/HTMLImageElement.h +++ b/dom/html/HTMLImageElement.h @@ -344,7 +344,7 @@ protected: void UpdateFormOwner(); virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index d5fca28f6..11ed3a5d4 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -1237,7 +1237,7 @@ HTMLInputElement::Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult) co nsresult HTMLInputElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None) { @@ -1452,8 +1452,6 @@ HTMLInputElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, // Clear the cached @autocomplete attribute state. mAutocompleteAttrState = nsContentUtils::eAutocompleteAttrState_Unknown; } - - UpdateState(aNotify); } return nsGenericHTMLFormElementWithState::AfterSetAttr(aNameSpaceID, aName, diff --git a/dom/html/HTMLInputElement.h b/dom/html/HTMLInputElement.h index 98c74893e..86bc7d030 100644 --- a/dom/html/HTMLInputElement.h +++ b/dom/html/HTMLInputElement.h @@ -958,7 +958,7 @@ protected: * Called when an attribute is about to be changed */ virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; /** * Called when an attribute has just been changed diff --git a/dom/html/HTMLLinkElement.cpp b/dom/html/HTMLLinkElement.cpp index 0a2cdaaf4..4c42d310a 100644 --- a/dom/html/HTMLLinkElement.cpp +++ b/dom/html/HTMLLinkElement.cpp @@ -333,7 +333,7 @@ HTMLLinkElement::UpdateImport() nsresult HTMLLinkElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, bool aNotify) + const nsAttrValueOrString* aValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None && (aName == nsGkAtoms::href || aName == nsGkAtoms::rel)) { diff --git a/dom/html/HTMLLinkElement.h b/dom/html/HTMLLinkElement.h index 1fa154f41..9c69cfb13 100644 --- a/dom/html/HTMLLinkElement.h +++ b/dom/html/HTMLLinkElement.h @@ -62,7 +62,7 @@ public: virtual void UnbindFromTree(bool aDeep = true, bool aNullParent = true) override; virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, diff --git a/dom/html/HTMLOptionElement.cpp b/dom/html/HTMLOptionElement.cpp index 7b1e3ca2d..9d861d414 100644 --- a/dom/html/HTMLOptionElement.cpp +++ b/dom/html/HTMLOptionElement.cpp @@ -181,7 +181,7 @@ HTMLOptionElement::GetAttributeChangeHint(const nsIAtom* aAttribute, nsresult HTMLOptionElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) { nsresult rv = nsGenericHTMLElement::BeforeSetAttr(aNamespaceID, aName, diff --git a/dom/html/HTMLOptionElement.h b/dom/html/HTMLOptionElement.h index 4b5e192ff..23ef2fb5e 100644 --- a/dom/html/HTMLOptionElement.h +++ b/dom/html/HTMLOptionElement.h @@ -47,7 +47,7 @@ public: int32_t aModType) const override; virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, bool aNotify) override; diff --git a/dom/html/HTMLSelectElement.cpp b/dom/html/HTMLSelectElement.cpp index 272a16578..a5f88f3fb 100644 --- a/dom/html/HTMLSelectElement.cpp +++ b/dom/html/HTMLSelectElement.cpp @@ -1269,7 +1269,7 @@ HTMLSelectElement::UnbindFromTree(bool aDeep, bool aNullParent) nsresult HTMLSelectElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) { if (aNotify && aName == nsGkAtoms::disabled && @@ -1294,8 +1294,6 @@ HTMLSelectElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, // Clear the cached @autocomplete attribute state mAutocompleteAttrState = nsContentUtils::eAutocompleteAttrState_Unknown; } - - UpdateState(aNotify); } return nsGenericHTMLFormElementWithState::AfterSetAttr(aNameSpaceID, aName, diff --git a/dom/html/HTMLSelectElement.h b/dom/html/HTMLSelectElement.h index 4cb5f90c9..8eb7fa410 100644 --- a/dom/html/HTMLSelectElement.h +++ b/dom/html/HTMLSelectElement.h @@ -375,7 +375,7 @@ public: bool aCompileEventHandlers) override; virtual void UnbindFromTree(bool aDeep, bool aNullParent) override; virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, bool aNotify) override; diff --git a/dom/html/HTMLTableElement.cpp b/dom/html/HTMLTableElement.cpp index c5b7696cf..5784fa691 100644 --- a/dom/html/HTMLTableElement.cpp +++ b/dom/html/HTMLTableElement.cpp @@ -986,7 +986,7 @@ HTMLTableElement::UnbindFromTree(bool aDeep, bool aNullParent) nsresult HTMLTableElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) { if (aName == nsGkAtoms::cellpadding && aNameSpaceID == kNameSpaceID_None) { diff --git a/dom/html/HTMLTableElement.h b/dom/html/HTMLTableElement.h index 4e172964b..cf83c9440 100644 --- a/dom/html/HTMLTableElement.h +++ b/dom/html/HTMLTableElement.h @@ -190,7 +190,7 @@ public: * Called when an attribute is about to be changed */ virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; /** * Called when an attribute has just been changed diff --git a/dom/html/HTMLTextAreaElement.cpp b/dom/html/HTMLTextAreaElement.cpp index 796d8a49e..c929daebd 100644 --- a/dom/html/HTMLTextAreaElement.cpp +++ b/dom/html/HTMLTextAreaElement.cpp @@ -1286,7 +1286,7 @@ HTMLTextAreaElement::UnbindFromTree(bool aDeep, bool aNullParent) nsresult HTMLTextAreaElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) { if (aNotify && aName == nsGkAtoms::disabled && @@ -1364,8 +1364,6 @@ HTMLTextAreaElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } else if (aName == nsGkAtoms::minlength) { UpdateTooShortValidityState(); } - - UpdateState(aNotify); } return nsGenericHTMLFormElementWithState::AfterSetAttr(aNameSpaceID, aName, aValue, diff --git a/dom/html/HTMLTextAreaElement.h b/dom/html/HTMLTextAreaElement.h index bfc7d203d..8723e85f6 100644 --- a/dom/html/HTMLTextAreaElement.h +++ b/dom/html/HTMLTextAreaElement.h @@ -144,7 +144,7 @@ public: * Called when an attribute is about to be changed */ virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; // nsIMutationObserver diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index 394a77a59..30ecd8508 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -1962,7 +1962,7 @@ nsGenericHTMLFormElement::UnbindFromTree(bool aDeep, bool aNullParent) nsresult nsGenericHTMLFormElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None) { @@ -1995,13 +1995,6 @@ nsGenericHTMLFormElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } mForm->RemoveElement(this, false); - - // Removing the element from the form can make it not be the default - // control anymore. Go ahead and notify on that change, though we might - // end up readding and becoming the default control again in - // AfterSetAttr. - // FIXME: Bug 656197 - UpdateState(aNotify); } if (aName == nsGkAtoms::form) { @@ -2051,12 +2044,6 @@ nsGenericHTMLFormElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } mForm->AddElement(this, false, aNotify); - - // Adding the element to the form can make it be the default control . - // Go ahead and notify on that change. - // Note: no need to notify on CanBeDisabled(), since type attr - // changes can't affect that. - UpdateState(aNotify); } if (aName == nsGkAtoms::form) { diff --git a/dom/html/nsGenericHTMLElement.h b/dom/html/nsGenericHTMLElement.h index ab4391aa1..a8a0d2596 100644 --- a/dom/html/nsGenericHTMLElement.h +++ b/dom/html/nsGenericHTMLElement.h @@ -1263,7 +1263,7 @@ protected: virtual ~nsGenericHTMLFormElement(); virtual nsresult BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, diff --git a/dom/svg/nsSVGElement.cpp b/dom/svg/nsSVGElement.cpp index ce849acf0..9099caaaa 100644 --- a/dom/svg/nsSVGElement.cpp +++ b/dom/svg/nsSVGElement.cpp @@ -1441,27 +1441,16 @@ nsSVGElement::WillChangeValue(nsIAtom* aName) // We need an empty attr value: // a) to pass to BeforeSetAttr when GetParsedAttr returns nullptr // b) to store the old value in the case we have mutation listeners - // We can use the same value for both purposes since (a) happens before (b). + // + // We can use the same value for both purposes, because if GetParsedAttr + // returns non-null its return value is what will get passed to BeforeSetAttr, + // not matter what our mutation listener situation is. + // // Also, we should be careful to always return this value to benefit from // return value optimization. nsAttrValue emptyOrOldAttrValue; const nsAttrValue* attrValue = GetParsedAttr(aName); - // This is not strictly correct--the attribute value parameter for - // BeforeSetAttr should reflect the value that *will* be set but that implies - // allocating, e.g. an extra nsSVGLength2, and isn't necessary at the moment - // since no SVG elements overload BeforeSetAttr. For now we just pass the - // current value. - nsAttrValueOrString attrStringOrValue(attrValue ? *attrValue - : emptyOrOldAttrValue); - DebugOnly rv = - BeforeSetAttr(kNameSpaceID_None, aName, &attrStringOrValue, - kNotifyDocumentObservers); - // SVG elements aren't expected to overload BeforeSetAttr in such a way that - // it may fail. So long as this is the case we don't need to check and pass on - // the return value which simplifies the calling code significantly. - MOZ_ASSERT(NS_SUCCEEDED(rv), "Unexpected failure from BeforeSetAttr"); - // We only need to set the old value if we have listeners since otherwise it // isn't used. if (attrValue && @@ -1477,6 +1466,21 @@ nsSVGElement::WillChangeValue(nsIAtom* aName) nsNodeUtils::AttributeWillChange(this, kNameSpaceID_None, aName, modType, nullptr); + // This is not strictly correct--the attribute value parameter for + // BeforeSetAttr should reflect the value that *will* be set but that implies + // allocating, e.g. an extra nsSVGLength2, and isn't necessary at the moment + // since no SVG elements overload BeforeSetAttr. For now we just pass the + // current value. + nsAttrValueOrString attrStringOrValue(attrValue ? *attrValue + : emptyOrOldAttrValue); + DebugOnly rv = + BeforeSetAttr(kNameSpaceID_None, aName, &attrStringOrValue, + kNotifyDocumentObservers); + // SVG elements aren't expected to overload BeforeSetAttr in such a way that + // it may fail. So long as this is the case we don't need to check and pass on + // the return value which simplifies the calling code significantly. + MOZ_ASSERT(NS_SUCCEEDED(rv), "Unexpected failure from BeforeSetAttr"); + return emptyOrOldAttrValue; } diff --git a/dom/svg/nsSVGElement.h b/dom/svg/nsSVGElement.h index 257ed7a2e..79278365a 100644 --- a/dom/svg/nsSVGElement.h +++ b/dom/svg/nsSVGElement.h @@ -329,7 +329,7 @@ protected: // BeforeSetAttr since it would involve allocating extra SVG value types. // See the comment in nsSVGElement::WillChangeValue. virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override final { return nsSVGElementBase::BeforeSetAttr(aNamespaceID, aName, aValue, aNotify); diff --git a/dom/xul/nsXULElement.cpp b/dom/xul/nsXULElement.cpp index c7a539370..e96bbeef4 100644 --- a/dom/xul/nsXULElement.cpp +++ b/dom/xul/nsXULElement.cpp @@ -1007,7 +1007,7 @@ nsXULElement::UnregisterAccessKey(const nsAString& aOldValue) nsresult nsXULElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, bool aNotify) + const nsAttrValueOrString* aValue, bool aNotify) { if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::accesskey && IsInUncomposedDoc()) { diff --git a/dom/xul/nsXULElement.h b/dom/xul/nsXULElement.h index 109f6c2e7..c2dcadf5f 100644 --- a/dom/xul/nsXULElement.h +++ b/dom/xul/nsXULElement.h @@ -624,7 +624,7 @@ protected: nsresult MakeHeavyweight(nsXULPrototypeElement* aPrototype); virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, - nsAttrValueOrString* aValue, + const nsAttrValueOrString* aValue, bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, const nsAttrValue* aValue, bool aNotify) override; -- cgit v1.2.3