From 17f7e1c8c6fca351174bdbd73981aa8e44d0f9da Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:33:06 -0400 Subject: Bug 1365092 - Move side effects of SetAttr and ParseAttribute to BeforeSetAttr and AfterSetAttr * Moves side effects of nsGenericHTMLElement and Element's SetAttr, UnsetAttr, and ParseAttribute functions to the corresponding BeforeSetAttr and AfterSetAttr functions * Moves side effects of HTMLAnchorElement's SetAttr, UnsetAttr, and ParseAttribute functions to the corresponding BeforeSetAttr and AfterSetAttr functions * Moves side effects of HTMLImageElement's SetAttr function to the corresponding BeforeSetAttr and AfterSetAttr functions * Moves side effects of SetAttr, UnsetAttr, and ParseAttribute functions to BeforeSetAttr and AfterSetAttr Tag #1375 --- dom/base/Element.cpp | 94 ++++++++++++++----- dom/base/Element.h | 66 ++++++++++++-- dom/base/nsAttrValueOrString.h | 14 +++ dom/base/nsStyledElement.cpp | 17 +++- dom/base/nsStyledElement.h | 4 + dom/html/HTMLAnchorElement.cpp | 86 ++++------------- dom/html/HTMLAnchorElement.h | 21 ++--- dom/html/HTMLAreaElement.cpp | 48 +++------- dom/html/HTMLAreaElement.h | 15 +-- dom/html/HTMLCanvasElement.cpp | 49 +++++----- dom/html/HTMLCanvasElement.h | 34 ++++--- dom/html/HTMLContentElement.cpp | 112 ++++++++++------------- dom/html/HTMLContentElement.h | 12 +-- dom/html/HTMLFormElement.cpp | 42 +++++---- dom/html/HTMLFormElement.h | 11 +-- dom/html/HTMLFrameSetElement.cpp | 61 +++++++------ dom/html/HTMLFrameSetElement.h | 15 +-- dom/html/HTMLIFrameElement.cpp | 67 +++++++------- dom/html/HTMLIFrameElement.h | 34 ++++--- dom/html/HTMLImageElement.cpp | 162 +++++++++++++++++++-------------- dom/html/HTMLImageElement.h | 45 ++++++--- dom/html/HTMLLegendElement.cpp | 15 --- dom/html/HTMLLegendElement.h | 10 -- dom/html/HTMLMediaElement.cpp | 112 +++++++++++------------ dom/html/HTMLMediaElement.h | 35 +++---- dom/html/HTMLObjectElement.cpp | 56 ++++++------ dom/html/HTMLObjectElement.h | 26 +++++- dom/html/HTMLSelectElement.cpp | 56 +++++------- dom/html/HTMLSelectElement.h | 2 - dom/html/HTMLSharedElement.cpp | 62 +++++-------- dom/html/HTMLSharedElement.h | 16 +--- dom/html/HTMLSharedObjectElement.cpp | 58 ++++++++---- dom/html/HTMLSharedObjectElement.h | 24 ++++- dom/html/nsGenericHTMLElement.cpp | 154 ++++++++++++++----------------- dom/html/nsGenericHTMLElement.h | 14 +-- dom/html/nsGenericHTMLFrameElement.cpp | 144 ++++++++++++++--------------- dom/html/nsGenericHTMLFrameElement.h | 35 ++++--- dom/mathml/nsMathMLElement.cpp | 45 +++------ dom/mathml/nsMathMLElement.h | 15 +-- dom/xbl/XBLChildrenElement.cpp | 39 +++----- dom/xbl/XBLChildrenElement.h | 12 +-- 41 files changed, 983 insertions(+), 956 deletions(-) (limited to 'dom') diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 11ad042a0..96ccfd52a 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -2476,7 +2476,7 @@ Element::SetAttr(int32_t aNamespaceID, nsIAtom* aName, if (OnlyNotifySameValueSet(aNamespaceID, aName, aPrefix, value, aNotify, oldValue, &modType, &hasListeners, &oldValueSet)) { - return NS_OK; + return OnAttrSetButNotChanged(aNamespaceID, aName, value, aNotify); } nsAttrValue attrValue; @@ -2494,20 +2494,21 @@ Element::SetAttr(int32_t aNamespaceID, nsIAtom* aName, 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 nsIDocument* document = GetComposedDoc(); mozAutoDocUpdate updateBatch(document, UPDATE_CONTENT_MODEL, aNotify); - // Even the value was pre-parsed, we still need to call ParseAttribute because - // it can have side effects. - if (!ParseAttribute(aNamespaceID, aName, aValue, attrValue)) { + nsresult rv = BeforeSetAttr(aNamespaceID, aName, &value, aNotify); + NS_ENSURE_SUCCESS(rv, rv); + + if (!preparsedAttrValue && + !ParseAttribute(aNamespaceID, aName, aValue, attrValue)) { attrValue.SetTo(aValue); } + PreIdMaybeChange(aNamespaceID, aName, &value); + return SetAttrAndNotify(aNamespaceID, aName, aPrefix, oldValueSet ? &oldValue : nullptr, attrValue, modType, hasListeners, aNotify, @@ -2538,7 +2539,7 @@ Element::SetParsedAttr(int32_t aNamespaceID, nsIAtom* aName, if (OnlyNotifySameValueSet(aNamespaceID, aName, aPrefix, value, aNotify, oldValue, &modType, &hasListeners, &oldValueSet)) { - return NS_OK; + return OnAttrSetButNotChanged(aNamespaceID, aName, value, aNotify); } if (aNotify) { @@ -2549,6 +2550,8 @@ Element::SetParsedAttr(int32_t aNamespaceID, nsIAtom* aName, nsresult rv = BeforeSetAttr(aNamespaceID, aName, &value, aNotify); NS_ENSURE_SUCCESS(rv, rv); + PreIdMaybeChange(aNamespaceID, aName, &value); + nsIDocument* document = GetComposedDoc(); mozAutoDocUpdate updateBatch(document, UPDATE_CONTENT_MODEL, aNotify); return SetAttrAndNotify(aNamespaceID, aName, aPrefix, @@ -2608,6 +2611,8 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, } NS_ENSURE_SUCCESS(rv, rv); + PostIdMaybeChange(aNamespaceID, aName, &valueForAfterSetAttr); + // If the old value owns its own data, we know it is OK to keep using it. // oldValue will be null if there was no previously set value const nsAttrValue* oldValue; @@ -2720,22 +2725,16 @@ Element::ParseAttribute(int32_t aNamespaceID, nsAttrValue& aResult) { if (aNamespaceID == kNameSpaceID_None) { - if (aAttribute == nsGkAtoms::_class) { - SetMayHaveClass(); - // Result should have been preparsed above. - return true; - } + MOZ_ASSERT(aAttribute != nsGkAtoms::_class, + "The class attribute should be preparsed and therefore should " + "never be passed to Element::ParseAttribute"); if (aAttribute == nsGkAtoms::id) { // Store id as an atom. id="" means that the element has no id, // not that it has an emptystring as the id. - RemoveFromIdTable(); if (aValue.IsEmpty()) { - ClearHasID(); return false; } aResult.ParseAtom(aValue); - SetHasID(); - AddToIdTable(aResult.GetAtomValue()); return true; } } @@ -2754,6 +2753,57 @@ Element::SetAndSwapMappedAttribute(nsIDocument* aDocument, return false; } +nsresult +Element::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, bool aNotify) +{ + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::_class) { + if (aValue) { + // Note: This flag is asymmetrical. It is never unset and isn't exact. + // If it is ever made to be exact, we probably need to handle this + // similarly to how ids are handled in PreIdMaybeChange and + // PostIdMaybeChange. + SetMayHaveClass(); + } + } + } + + return NS_OK; +} + +void +Element::PreIdMaybeChange(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue) +{ + if (aNamespaceID != kNameSpaceID_None || aName != nsGkAtoms::id) { + return; + } + RemoveFromIdTable(); + + return; +} + +void +Element::PostIdMaybeChange(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue) +{ + if (aNamespaceID != kNameSpaceID_None || aName != nsGkAtoms::id) { + return; + } + + // id="" means that the element has no id, not that it has an empty + // string as the id. + if (aValue && !aValue->IsEmptyString()) { + SetHasID(); + AddToIdTable(aValue->GetAtomValue()); + } else { + ClearHasID(); + } + + return; +} + EventListenerManager* Element::GetEventListenerManagerForAttr(nsIAtom* aAttrName, bool* aDefer) @@ -2847,6 +2897,8 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, NS_EVENT_BITS_MUTATION_ATTRMODIFIED, this); + PreIdMaybeChange(aNameSpaceID, aName, nullptr); + // Grab the attr node if needed before we remove it from the attr map RefPtr attrNode; if (hasMutationListeners) { @@ -2865,12 +2917,6 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, // react to unexpected attribute changes. nsMutationGuard::DidMutate(); - if (aName == nsGkAtoms::id && aNameSpaceID == kNameSpaceID_None) { - // Have to do this before clearing flag. See RemoveFromIdTable - RemoveFromIdTable(); - ClearHasID(); - } - bool hadValidDir = false; bool hadDirAuto = false; @@ -2883,6 +2929,8 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, rv = mAttrsAndChildren.RemoveAttrAt(index, oldValue); NS_ENSURE_SUCCESS(rv, rv); + PostIdMaybeChange(aNameSpaceID, aName, nullptr); + if (document || HasFlag(NODE_FORCE_XBL_BINDINGS)) { RefPtr binding = GetXBLBinding(); if (binding) { diff --git a/dom/base/Element.h b/dom/base/Element.h index 6f60c9ad0..df0dbcc45 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -523,6 +523,7 @@ public: already_AddRefed GetExistingAttrNameFromQName(const nsAString& aStr) const; + MOZ_ALWAYS_INLINE // Avoid a crashy hook from Avast 10 Beta (Bug 1058131) nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAString& aValue, bool aNotify) { @@ -1378,14 +1379,9 @@ protected: * 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, const nsAttrValueOrString* aValue, - bool aNotify) - { - return NS_OK; - } + bool aNotify); /** * Hook that is called by Element::SetAttr to allow subclasses to @@ -1412,6 +1408,64 @@ protected: return NS_OK; } + /** + * This function shall be called just before the id attribute changes. It will + * be called after BeforeSetAttr. If the attribute being changed is not the id + * attribute, this function does nothing. Otherwise, it will remove the old id + * from the document's id cache. + * + * This must happen after BeforeSetAttr (rather than during) because the + * the subclasses' calls to BeforeSetAttr may notify on state changes. If they + * incorrectly determine whether the element had an id, the element may not be + * restyled properly. + * + * @param aNamespaceID the namespace of the attr being set + * @param aName the localname of the attribute being set + * @param aValue the new id value. Will be null if the id is being unset. + */ + void PreIdMaybeChange(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue); + + /** + * This function shall be called just after the id attribute changes. It will + * be called before AfterSetAttr. If the attribute being changed is not the id + * attribute, this function does nothing. Otherwise, it will add the new id to + * the document's id cache and properly set the ElementHasID flag. + * + * This must happen before AfterSetAttr (rather than during) because the + * the subclasses' calls to AfterSetAttr may notify on state changes. If they + * incorrectly determine whether the element now has an id, the element may + * not be restyled properly. + * + * @param aNamespaceID the namespace of the attr being set + * @param aName the localname of the attribute being set + * @param aValue the new id value. Will be null if the id is being unset. + */ + void PostIdMaybeChange(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue); + + /** + * Usually, setting an attribute to the value that it already has results in + * no action. However, in some cases, setting an attribute to its current + * value should have the effect of, for example, forcing a reload of + * network data. To address that, this function will be called in this + * situation to allow the handling of such a case. + * + * @param aNamespaceID the namespace of the attr being set + * @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. + * @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 OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) + { + return NS_OK; + } + /** * Hook to allow subclasses to produce a different EventListenerManager if * needed for attachment of attribute-defined handlers diff --git a/dom/base/nsAttrValueOrString.h b/dom/base/nsAttrValueOrString.h index 8e6c06953..1a62a8428 100644 --- a/dom/base/nsAttrValueOrString.h +++ b/dom/base/nsAttrValueOrString.h @@ -78,6 +78,20 @@ public: return aOther.EqualsAsStrings(*mAttrValue); } + /* + * Returns true if the value stored is empty + */ + bool IsEmpty() const + { + if (mStringPtr) { + return mStringPtr->IsEmpty(); + } + if (mAttrValue) { + return mAttrValue->IsEmptyString(); + } + return true; + } + protected: const nsAttrValue* mAttrValue; mutable const nsAString* mStringPtr; diff --git a/dom/base/nsStyledElement.cpp b/dom/base/nsStyledElement.cpp index e3d8dfe57..8cd448d47 100644 --- a/dom/base/nsStyledElement.cpp +++ b/dom/base/nsStyledElement.cpp @@ -40,7 +40,6 @@ nsStyledElement::ParseAttribute(int32_t aNamespaceID, nsAttrValue& aResult) { if (aAttribute == nsGkAtoms::style && aNamespaceID == kNameSpaceID_None) { - SetMayHaveStyle(); ParseStyleAttribute(aValue, aResult, false); return true; } @@ -49,6 +48,22 @@ nsStyledElement::ParseAttribute(int32_t aNamespaceID, aResult); } +nsresult +nsStyledElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, bool aNotify) +{ + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::style) { + if (aValue) { + SetMayHaveStyle(); + } + } + } + + return nsStyledElementBase::BeforeSetAttr(aNamespaceID, aName, aValue, + aNotify); +} + nsresult nsStyledElement::SetInlineStyleDeclaration(DeclarationBlock* aDeclaration, const nsAString* aSerialized, diff --git a/dom/base/nsStyledElement.h b/dom/base/nsStyledElement.h index c4894d87f..cb6dfd54b 100644 --- a/dom/base/nsStyledElement.h +++ b/dom/base/nsStyledElement.h @@ -80,6 +80,10 @@ protected: * document. */ nsresult ReparseStyleAttribute(bool aForceInDataDoc); + + virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) override; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsStyledElement, NS_STYLED_ELEMENT_IID) diff --git a/dom/html/HTMLAnchorElement.cpp b/dom/html/HTMLAnchorElement.cpp index 501d6e3d0..0759af339 100644 --- a/dom/html/HTMLAnchorElement.cpp +++ b/dom/html/HTMLAnchorElement.cpp @@ -14,6 +14,7 @@ #include "nsContentUtils.h" #include "nsGkAtoms.h" #include "nsHTMLDNSPrefetch.h" +#include "nsAttrValueOrString.h" #include "nsIDocument.h" #include "nsIPresShell.h" #include "nsPresContext.h" @@ -372,84 +373,37 @@ HTMLAnchorElement::GetHrefURI() const } nsresult -HTMLAnchorElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) +HTMLAnchorElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) { - bool reset = false; - if (aName == nsGkAtoms::href && kNameSpaceID_None == aNameSpaceID) { - // If we do not have a cached URI, we have some value here so we must reset - // our link state after calling the parent. - if (!Link::HasCachedURI()) { - reset = true; - } - // However, if we have a cached URI, we'll want to see if the value changed. - else { - nsAutoString val; - GetHref(val); - if (!val.Equals(aValue)) { - reset = true; - } - } - if (reset) { + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::href) { CancelDNSPrefetch(HTML_ANCHOR_DNS_PREFETCH_DEFERRED, HTML_ANCHOR_DNS_PREFETCH_REQUESTED); } } - nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, - aValue, aNotify); - - // The ordering of the parent class's SetAttr call and Link::ResetLinkState - // is important here! The attribute is not set until SetAttr returns, and - // we will need the updated attribute value because notifying the document - // that content states have changed will call IntrinsicState, which will try - // to get updated information about the visitedness from Link. - if (reset) { - Link::ResetLinkState(!!aNotify, true); - if (IsInComposedDoc()) { - TryDNSPrefetch(); - } - } - - return rv; + return nsGenericHTMLElement::BeforeSetAttr(aNamespaceID, aName, aValue, + aNotify); } nsresult -HTMLAnchorElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) +HTMLAnchorElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { - bool href = - (aAttribute == nsGkAtoms::href && kNameSpaceID_None == aNameSpaceID); - - if (href) { - CancelDNSPrefetch(HTML_ANCHOR_DNS_PREFETCH_DEFERRED, - HTML_ANCHOR_DNS_PREFETCH_REQUESTED); - } - - nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute, - aNotify); - - // The ordering of the parent class's UnsetAttr call and Link::ResetLinkState - // is important here! The attribute is not unset until UnsetAttr returns, and - // we will need the updated attribute value because notifying the document - // that content states have changed will call IntrinsicState, which will try - // to get updated information about the visitedness from Link. - if (href) { - Link::ResetLinkState(!!aNotify, false); + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::href) { + Link::ResetLinkState(aNotify, !!aValue); + if (aValue && IsInComposedDoc()) { + TryDNSPrefetch(); + } + } } - return rv; -} - -bool -HTMLAnchorElement::ParseAttribute(int32_t aNamespaceID, - nsIAtom* aAttribute, - const nsAString& aValue, - nsAttrValue& aResult) -{ - return nsGenericHTMLElement::ParseAttribute(aNamespaceID, aAttribute, aValue, - aResult); + return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, + aValue, aOldValue, aNotify); } EventStates diff --git a/dom/html/HTMLAnchorElement.h b/dom/html/HTMLAnchorElement.h index 027d8d689..087555964 100644 --- a/dom/html/HTMLAnchorElement.h +++ b/dom/html/HTMLAnchorElement.h @@ -66,20 +66,13 @@ public: virtual void GetLinkTarget(nsAString& aTarget) override; virtual already_AddRefed GetHrefURI() const override; - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; - virtual bool ParseAttribute(int32_t aNamespaceID, - nsIAtom* aAttribute, - const nsAString& aValue, - nsAttrValue& aResult) override; + virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) override; + virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult) const override; diff --git a/dom/html/HTMLAreaElement.cpp b/dom/html/HTMLAreaElement.cpp index 213d4831d..4c85774f4 100644 --- a/dom/html/HTMLAreaElement.cpp +++ b/dom/html/HTMLAreaElement.cpp @@ -153,42 +153,22 @@ HTMLAreaElement::UnbindFromTree(bool aDeep, bool aNullParent) } nsresult -HTMLAreaElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) -{ - nsresult rv = - nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, aValue, aNotify); - - // The ordering of the parent class's SetAttr call and Link::ResetLinkState - // is important here! The attribute is not set until SetAttr returns, and - // we will need the updated attribute value because notifying the document - // that content states have changed will call IntrinsicState, which will try - // to get updated information about the visitedness from Link. - if (aName == nsGkAtoms::href && aNameSpaceID == kNameSpaceID_None) { - Link::ResetLinkState(!!aNotify, true); +HTMLAreaElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) +{ + if (aNamespaceID == kNameSpaceID_None) { + // This must happen after the attribute is set. We will need the updated + // attribute value because notifying the document that content states have + // changed will call IntrinsicState, which will try to get updated + // information about the visitedness from Link. + if (aName == nsGkAtoms::href) { + Link::ResetLinkState(aNotify, !!aValue); + } } - return rv; -} - -nsresult -HTMLAreaElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) -{ - nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute, - aNotify); - - // The ordering of the parent class's UnsetAttr call and Link::ResetLinkState - // is important here! The attribute is not unset until UnsetAttr returns, and - // we will need the updated attribute value because notifying the document - // that content states have changed will call IntrinsicState, which will try - // to get updated information about the visitedness from Link. - if (aAttribute == nsGkAtoms::href && kNameSpaceID_None == aNameSpaceID) { - Link::ResetLinkState(!!aNotify, false); - } - - return rv; + return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue, + aOldValue, aNotify); } #define IMPL_URI_PART(_part) \ diff --git a/dom/html/HTMLAreaElement.h b/dom/html/HTMLAreaElement.h index c2ed7a928..d408934c9 100644 --- a/dom/html/HTMLAreaElement.h +++ b/dom/html/HTMLAreaElement.h @@ -56,16 +56,6 @@ public: bool aCompileEventHandlers) override; virtual void UnbindFromTree(bool aDeep = true, bool aNullParent = true) override; - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; virtual nsresult Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult) const override; @@ -186,6 +176,11 @@ protected: virtual JSObject* WrapNode(JSContext* aCx, JS::Handle aGivenProto) override; + virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; + RefPtr mRelList; }; diff --git a/dom/html/HTMLCanvasElement.cpp b/dom/html/HTMLCanvasElement.cpp index 803c60e8a..ba1fbedbb 100644 --- a/dom/html/HTMLCanvasElement.cpp +++ b/dom/html/HTMLCanvasElement.cpp @@ -446,38 +446,37 @@ NS_IMPL_UINT_ATTR_DEFAULT_VALUE(HTMLCanvasElement, Height, height, DEFAULT_CANVA NS_IMPL_BOOL_ATTR(HTMLCanvasElement, MozOpaque, moz_opaque) nsresult -HTMLCanvasElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) -{ - nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, aValue, - aNotify); - if (NS_SUCCEEDED(rv) && mCurrentContext && - aNameSpaceID == kNameSpaceID_None && - (aName == nsGkAtoms::width || aName == nsGkAtoms::height || aName == nsGkAtoms::moz_opaque)) - { - ErrorResult dummy; - rv = UpdateContext(nullptr, JS::NullHandleValue, dummy); - NS_ENSURE_SUCCESS(rv, rv); - } +HTMLCanvasElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) +{ + AfterMaybeChangeAttr(aNamespaceID, aName, aNotify); - return rv; + return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue, + aOldValue, aNotify); } nsresult -HTMLCanvasElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, - bool aNotify) +HTMLCanvasElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) { - nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aName, aNotify); - if (NS_SUCCEEDED(rv) && mCurrentContext && - aNameSpaceID == kNameSpaceID_None && - (aName == nsGkAtoms::width || aName == nsGkAtoms::height || aName == nsGkAtoms::moz_opaque)) - { + AfterMaybeChangeAttr(aNamespaceID, aName, aNotify); + + return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName, + aValue, aNotify); +} + +void +HTMLCanvasElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + bool aNotify) +{ + if (mCurrentContext && aNamespaceID == kNameSpaceID_None && + (aName == nsGkAtoms::width || aName == nsGkAtoms::height || + aName == nsGkAtoms::moz_opaque)) { ErrorResult dummy; - rv = UpdateContext(nullptr, JS::NullHandleValue, dummy); - NS_ENSURE_SUCCESS(rv, rv); + UpdateContext(nullptr, JS::NullHandleValue, dummy); } - return rv; } void diff --git a/dom/html/HTMLCanvasElement.h b/dom/html/HTMLCanvasElement.h index de26c475a..822f05397 100644 --- a/dom/html/HTMLCanvasElement.h +++ b/dom/html/HTMLCanvasElement.h @@ -295,20 +295,6 @@ public: nsAttrValue& aResult) override; nsChangeHint GetAttributeChangeHint(const nsIAtom* aAttribute, int32_t aModType) const override; - // SetAttr override. C++ is stupid, so have to override both - // overloaded methods. - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, - bool aNotify) override; - virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult) const override; nsresult CopyInnerTo(mozilla::dom::Element* aDest); @@ -373,6 +359,14 @@ protected: nsISupports** aResult); void CallPrintCallback(); + virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; + virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) override; + AsyncCanvasRenderer* GetAsyncCanvasRenderer(); bool mResetLayer; @@ -406,6 +400,18 @@ public: CanvasContextType GetCurrentContextType() { return mCurrentContextType; } + +private: + /** + * This function is called by AfterSetAttr and OnAttrSetButNotChanged. + * This function will be called by AfterSetAttr whether the attribute is being + * set or unset. + * + * @param aNamespaceID the namespace of the attr being set + * @param aName the localname of the attribute being set + * @param aNotify Whether we plan to notify document observers. + */ + void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, bool aNotify); }; class HTMLCanvasPrintState final : public nsWrapperCache diff --git a/dom/html/HTMLContentElement.cpp b/dom/html/HTMLContentElement.cpp index 01c0158a0..158f8d0ce 100644 --- a/dom/html/HTMLContentElement.cpp +++ b/dom/html/HTMLContentElement.cpp @@ -210,75 +210,63 @@ IsValidContentSelectors(nsCSSSelector* aSelector) } nsresult -HTMLContentElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) +HTMLContentElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { - nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, - aValue, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::select) { - // Select attribute was updated, the insertion point may match different - // elements. - nsIDocument* doc = OwnerDoc(); - nsCSSParser parser(doc->CSSLoader()); - - mValidSelector = true; - mSelectorList = nullptr; - - nsresult rv = parser.ParseSelectorString(aValue, - doc->GetDocumentURI(), - // Bug 11240 - 0, // XXX get the line number! - getter_Transfers(mSelectorList)); - - // We don't want to return an exception if parsing failed because - // the spec does not define it as an exception case. - if (NS_SUCCEEDED(rv)) { - // Ensure that all the selectors are valid - nsCSSSelectorList* selectors = mSelectorList; - while (selectors) { - if (!IsValidContentSelectors(selectors->mSelectors)) { - // If we have an invalid selector, we can not match anything. - mValidSelector = false; - mSelectorList = nullptr; - break; + if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::select) { + if (aValue) { + // Select attribute was updated, the insertion point may match different + // elements. + nsIDocument* doc = OwnerDoc(); + nsCSSParser parser(doc->CSSLoader()); + + mValidSelector = true; + mSelectorList = nullptr; + + nsAutoString valueStr; + aValue->ToString(valueStr); + nsresult rv = parser.ParseSelectorString(valueStr, + doc->GetDocumentURI(), + // Bug 11240 + 0, // XXX get the line number! + getter_Transfers(mSelectorList)); + + // We don't want to return an exception if parsing failed because + // the spec does not define it as an exception case. + if (NS_SUCCEEDED(rv)) { + // Ensure that all the selectors are valid + nsCSSSelectorList* selectors = mSelectorList; + while (selectors) { + if (!IsValidContentSelectors(selectors->mSelectors)) { + // If we have an invalid selector, we can not match anything. + mValidSelector = false; + mSelectorList = nullptr; + break; + } + selectors = selectors->mNext; } - selectors = selectors->mNext; } - } - - ShadowRoot* containingShadow = GetContainingShadow(); - if (containingShadow) { - containingShadow->DistributeAllNodes(); - } - } - - return NS_OK; -} -nsresult -HTMLContentElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) -{ - nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, - aAttribute, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - if (aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::select) { - // The select attribute was removed. This insertion point becomes - // a universal selector. - mValidSelector = true; - mSelectorList = nullptr; - - ShadowRoot* containingShadow = GetContainingShadow(); - if (containingShadow) { - containingShadow->DistributeAllNodes(); + ShadowRoot* containingShadow = GetContainingShadow(); + if (containingShadow) { + containingShadow->DistributeAllNodes(); + } + } else { + // The select attribute was removed. This insertion point becomes + // a universal selector. + mValidSelector = true; + mSelectorList = nullptr; + + ShadowRoot* containingShadow = GetContainingShadow(); + if (containingShadow) { + containingShadow->DistributeAllNodes(); + } } } - return NS_OK; + return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue, + aOldValue, aNotify); } bool diff --git a/dom/html/HTMLContentElement.h b/dom/html/HTMLContentElement.h index f019e56cd..d2491a792 100644 --- a/dom/html/HTMLContentElement.h +++ b/dom/html/HTMLContentElement.h @@ -63,13 +63,6 @@ public: void InsertMatchedNode(uint32_t aIndex, nsIContent* aContent); void ClearMatchedNodes(); - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; - // WebIDL methods. already_AddRefed GetDistributedNodes(); void GetSelect(nsAString& aSelect) @@ -95,6 +88,11 @@ protected: */ void UpdateFallbackDistribution(); + virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; + /** * An array of nodes from the ShadowRoot host that match the * content insertion selector. diff --git a/dom/html/HTMLFormElement.cpp b/dom/html/HTMLFormElement.cpp index 1fdfb602e..58cba6863 100644 --- a/dom/html/HTMLFormElement.cpp +++ b/dom/html/HTMLFormElement.cpp @@ -189,27 +189,31 @@ HTMLFormElement::GetElements(nsIDOMHTMLCollection** aElements) } nsresult -HTMLFormElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) -{ - if ((aName == nsGkAtoms::action || aName == nsGkAtoms::target) && - aNameSpaceID == kNameSpaceID_None) { - if (mPendingSubmission) { - // aha, there is a pending submission that means we're in - // the script and we need to flush it. let's tell it - // that the event was ignored to force the flush. - // the second argument is not playing a role at all. - FlushPendingSubmission(); +HTMLFormElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, bool aNotify) +{ + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::action || aName == nsGkAtoms::target) { + // This check is mostly to preserve previous behavior. + if (aValue) { + if (mPendingSubmission) { + // aha, there is a pending submission that means we're in + // the script and we need to flush it. let's tell it + // that the event was ignored to force the flush. + // the second argument is not playing a role at all. + FlushPendingSubmission(); + } + // Don't forget we've notified the password manager already if the + // page sets the action/target in the during submit. (bug 343182) + bool notifiedObservers = mNotifiedObservers; + ForgetCurrentSubmission(); + mNotifiedObservers = notifiedObservers; + } } - // Don't forget we've notified the password manager already if the - // page sets the action/target in the during submit. (bug 343182) - bool notifiedObservers = mNotifiedObservers; - ForgetCurrentSubmission(); - mNotifiedObservers = notifiedObservers; } - return nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, aValue, - aNotify); + + return nsGenericHTMLElement::BeforeSetAttr(aNamespaceID, aName, aValue, + aNotify); } nsresult diff --git a/dom/html/HTMLFormElement.h b/dom/html/HTMLFormElement.h index 6102ea61f..f06db4b51 100644 --- a/dom/html/HTMLFormElement.h +++ b/dom/html/HTMLFormElement.h @@ -105,14 +105,9 @@ public: bool aCompileEventHandlers) override; virtual void UnbindFromTree(bool aDeep = true, bool aNullParent = true) override; - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; + virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, const nsAttrValue* aOldValue, diff --git a/dom/html/HTMLFrameSetElement.cpp b/dom/html/HTMLFrameSetElement.cpp index 6d39caa19..861dbfe9f 100644 --- a/dom/html/HTMLFrameSetElement.cpp +++ b/dom/html/HTMLFrameSetElement.cpp @@ -9,6 +9,7 @@ #include "mozilla/dom/EventHandlerBinding.h" #include "nsGlobalWindow.h" #include "mozilla/UniquePtrExtensions.h" +#include "nsAttrValueOrString.h" NS_IMPL_NS_NEW_HTML_ELEMENT(FrameSet) @@ -65,43 +66,45 @@ HTMLFrameSetElement::GetRows(nsAString& aRows) } nsresult -HTMLFrameSetElement::SetAttr(int32_t aNameSpaceID, - nsIAtom* aAttribute, - nsIAtom* aPrefix, - const nsAString& aValue, - bool aNotify) +HTMLFrameSetElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) { - nsresult rv; /* The main goal here is to see whether the _number_ of rows or - * columns has changed. If it has, we need to reframe; otherwise - * we want to reflow. So we set mCurrentRowColHint here, then call - * nsGenericHTMLElement::SetAttr, which will end up calling - * GetAttributeChangeHint and notifying layout with that hint. - * Once nsGenericHTMLElement::SetAttr returns, we want to go back to our - * normal hint, which is NS_STYLE_HINT_REFLOW. + * columns has changed. If it has, we need to reframe; otherwise + * we want to reflow. + * Ideally, the style hint would be changed back to reflow after the reframe + * has been performed. Unfortunately, however, the reframe will be performed + * by the call to nsNodeUtils::AttributeChanged, which occurs *after* + * AfterSetAttr is called, leaving us with no convenient way of changing the + * value back to reflow afterwards. However, nsNodeUtils::AttributeChanged is + * effectively the only consumer of this value, so as long as we always set + * the value correctly here, we should be fine. */ - if (aAttribute == nsGkAtoms::rows && aNameSpaceID == kNameSpaceID_None) { - int32_t oldRows = mNumRows; - ParseRowCol(aValue, mNumRows, &mRowSpecs); + mCurrentRowColHint = NS_STYLE_HINT_REFLOW; + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::rows) { + if (aValue) { + int32_t oldRows = mNumRows; + ParseRowCol(aValue->String(), mNumRows, &mRowSpecs); - if (mNumRows != oldRows) { - mCurrentRowColHint = nsChangeHint_ReconstructFrame; - } - } else if (aAttribute == nsGkAtoms::cols && - aNameSpaceID == kNameSpaceID_None) { - int32_t oldCols = mNumCols; - ParseRowCol(aValue, mNumCols, &mColSpecs); + if (mNumRows != oldRows) { + mCurrentRowColHint = nsChangeHint_ReconstructFrame; + } + } + } else if (aName == nsGkAtoms::cols) { + if (aValue) { + int32_t oldCols = mNumCols; + ParseRowCol(aValue->String(), mNumCols, &mColSpecs); - if (mNumCols != oldCols) { - mCurrentRowColHint = nsChangeHint_ReconstructFrame; + if (mNumCols != oldCols) { + mCurrentRowColHint = nsChangeHint_ReconstructFrame; + } + } } } - rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aAttribute, aPrefix, - aValue, aNotify); - mCurrentRowColHint = NS_STYLE_HINT_REFLOW; - - return rv; + return nsGenericHTMLElement::BeforeSetAttr(aNamespaceID, aName, aValue, aNotify); } nsresult diff --git a/dom/html/HTMLFrameSetElement.h b/dom/html/HTMLFrameSetElement.h index b6bbe5d95..fe3bc2341 100644 --- a/dom/html/HTMLFrameSetElement.h +++ b/dom/html/HTMLFrameSetElement.h @@ -100,17 +100,6 @@ public: #undef WINDOW_EVENT_HELPER #undef EVENT - // These override the SetAttr methods in nsGenericHTMLElement (need - // both here to silence compiler warnings). - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - /** * GetRowSpec is used to get the "rows" spec. * @param out int32_t aNumValues The number of row sizes specified. @@ -143,6 +132,10 @@ protected: virtual JSObject* WrapNode(JSContext *aCx, JS::Handle aGivenProto) override; + virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) override; + private: nsresult ParseRowCol(const nsAString& aValue, int32_t& aNumSpecs, diff --git a/dom/html/HTMLIFrameElement.cpp b/dom/html/HTMLIFrameElement.cpp index 6ae4ec9dd..2b66cbd26 100644 --- a/dom/html/HTMLIFrameElement.cpp +++ b/dom/html/HTMLIFrameElement.cpp @@ -181,55 +181,50 @@ HTMLIFrameElement::GetAttributeMappingFunction() const return &MapAttributesIntoRule; } -nsresult -HTMLIFrameElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) -{ - nsresult rv = nsGenericHTMLFrameElement::SetAttr(aNameSpaceID, aName, - aPrefix, aValue, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::srcdoc) { - // Don't propagate error here. The attribute was successfully set, that's - // what we should reflect. - LoadSrc(); - } - - return NS_OK; -} - nsresult HTMLIFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, const nsAttrValue* aOldValue, bool aNotify) { - if (aName == nsGkAtoms::sandbox && - aNameSpaceID == kNameSpaceID_None && mFrameLoader) { - // If we have an nsFrameLoader, apply the new sandbox flags. - // Since this is called after the setter, the sandbox flags have - // alreay been updated. - mFrameLoader->ApplySandboxFlags(GetSandboxFlags()); + AfterMaybeChangeAttr(aNameSpaceID, aName, aNotify); + + if (aNameSpaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::sandbox) { + if (mFrameLoader) { + // If we have an nsFrameLoader, apply the new sandbox flags. + // Since this is called after the setter, the sandbox flags have + // alreay been updated. + mFrameLoader->ApplySandboxFlags(GetSandboxFlags()); + } + } } return nsGenericHTMLFrameElement::AfterSetAttr(aNameSpaceID, aName, aValue, aOldValue, aNotify); } nsresult -HTMLIFrameElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) +HTMLIFrameElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) { - // Invoke on the superclass. - nsresult rv = nsGenericHTMLFrameElement::UnsetAttr(aNameSpaceID, aAttribute, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - if (aNameSpaceID == kNameSpaceID_None && - aAttribute == nsGkAtoms::srcdoc) { - // Fall back to the src attribute, if any - LoadSrc(); - } + AfterMaybeChangeAttr(aNamespaceID, aName, aNotify); + + return nsGenericHTMLFrameElement::OnAttrSetButNotChanged(aNamespaceID, aName, + aValue, aNotify); +} - return NS_OK; +void +HTMLIFrameElement::AfterMaybeChangeAttr(int32_t aNamespaceID, + nsIAtom* aName, + bool aNotify) +{ + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::srcdoc) { + // Don't propagate errors from LoadSrc. The attribute was successfully + // set/unset, that's what we should reflect. + LoadSrc(); + } + } } uint32_t diff --git a/dom/html/HTMLIFrameElement.h b/dom/html/HTMLIFrameElement.h index 23afaaddd..e94ae0bad 100644 --- a/dom/html/HTMLIFrameElement.h +++ b/dom/html/HTMLIFrameElement.h @@ -46,21 +46,6 @@ public: virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult) const override; - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, - const nsAttrValue* aOldValue, - bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; - uint32_t GetSandboxFlags(); // Web IDL binding methods @@ -197,11 +182,30 @@ protected: virtual JSObject* WrapNode(JSContext* aCx, JS::Handle aGivenProto) override; + virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; + virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) override; + private: static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes, nsRuleData* aData); static const DOMTokenListSupportedToken sSupportedSandboxTokens[]; + + /** + * This function is called by AfterSetAttr and OnAttrSetButNotChanged. + * This function will be called by AfterSetAttr whether the attribute is being + * set or unset. + * + * @param aNamespaceID the namespace of the attr being set + * @param aName the localname of the attribute being set + * @param aNotify Whether we plan to notify document observers. + */ + void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, bool aNotify); }; } // namespace dom diff --git a/dom/html/HTMLImageElement.cpp b/dom/html/HTMLImageElement.cpp index dd43ac36a..444c352e2 100644 --- a/dom/html/HTMLImageElement.cpp +++ b/dom/html/HTMLImageElement.cpp @@ -112,6 +112,7 @@ private: HTMLImageElement::HTMLImageElement(already_AddRefed& aNodeInfo) : nsGenericHTMLElement(aNodeInfo) , mForm(nullptr) + , mForceReload(false) , mInDocResponsiveContent(false) , mCurrentDensity(1.0) { @@ -372,6 +373,9 @@ HTMLImageElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValueOrString* aValue, bool aNotify) { + if (aValue) { + BeforeMaybeChangeAttr(aNameSpaceID, aName, *aValue, aNotify); + } if (aNameSpaceID == kNameSpaceID_None && mForm && (aName == nsGkAtoms::name || aName == nsGkAtoms::id)) { @@ -394,6 +398,10 @@ HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, const nsAttrValue* aOldValue, bool aNotify) { + if (aValue) { + AfterMaybeChangeAttr(aNameSpaceID, aName, aNotify); + } + if (aNameSpaceID == kNameSpaceID_None && mForm && (aName == nsGkAtoms::name || aName == nsGkAtoms::id) && aValue && !aValue->IsEmptyString()) { @@ -438,63 +446,22 @@ HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } nsresult -HTMLImageElement::GetEventTargetParent(EventChainPreVisitor& aVisitor) -{ - // We handle image element with attribute ismap in its corresponding frame - // element. Set mMultipleActionsPrevented here to prevent the click event - // trigger the behaviors in Element::PostHandleEventForLinks - WidgetMouseEvent* mouseEvent = aVisitor.mEvent->AsMouseEvent(); - if (mouseEvent && mouseEvent->IsLeftClickEvent() && IsMap()) { - mouseEvent->mFlags.mMultipleActionsPrevented = true; - } - return nsGenericHTMLElement::GetEventTargetParent(aVisitor); -} - -bool -HTMLImageElement::IsHTMLFocusable(bool aWithMouse, - bool *aIsFocusable, int32_t *aTabIndex) +HTMLImageElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) { - int32_t tabIndex = TabIndex(); - - if (IsInUncomposedDoc()) { - nsAutoString usemap; - GetUseMap(usemap); - // XXXbz which document should this be using? sXBL/XBL2 issue! I - // think that OwnerDoc() is right, since we don't want to - // assume stuff about the document we're bound to. - if (OwnerDoc()->FindImageMap(usemap)) { - if (aTabIndex) { - // Use tab index on individual map areas - *aTabIndex = (sTabFocusModel & eTabFocus_linksMask)? 0 : -1; - } - // Image map is not focusable itself, but flag as tabbable - // so that image map areas get walked into. - *aIsFocusable = false; - - return false; - } - } - - if (aTabIndex) { - // Can be in tab order if tabindex >=0 and form controls are tabbable. - *aTabIndex = (sTabFocusModel & eTabFocus_formElementsMask)? tabIndex : -1; - } - - *aIsFocusable = -#ifdef XP_MACOSX - (!aWithMouse || nsFocusManager::sMouseFocusesFormControl) && -#endif - (tabIndex >= 0 || HasAttr(kNameSpaceID_None, nsGkAtoms::tabindex)); + BeforeMaybeChangeAttr(aNamespaceID, aName, aValue, aNotify); + AfterMaybeChangeAttr(aNamespaceID, aName, aNotify); - return false; + return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName, + aValue, aNotify); } -nsresult -HTMLImageElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) +void +HTMLImageElement::BeforeMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) { - bool forceReload = false; // We need to force our image to reload. This must be done here, not in // AfterSetAttr or BeforeSetAttr, because we want to do it even if the attr is // being set to its existing value, which is normally optimized away as a @@ -505,16 +472,19 @@ HTMLImageElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, // spec. // // Both cases handle unsetting src in AfterSetAttr - if (aNameSpaceID == kNameSpaceID_None && + // + // Much of this should probably happen in AfterMaybeChangeAttr. + // See Bug 1370705 + if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::src) { if (InResponsiveMode()) { if (mResponsiveSelector && mResponsiveSelector->Content() == this) { - mResponsiveSelector->SetDefaultSource(aValue); + mResponsiveSelector->SetDefaultSource(aValue.String()); } QueueImageLoadTask(true); - } else if (aNotify) { + } else if (aNotify && OwnerDoc()->IsCurrentActiveDocument()) { // If aNotify is false, we are coming from the parser or some such place; // we'll get bound after all the attributes have been set, so we'll do the // sync image load from BindToTree. Skip the LoadImage call in that case. @@ -529,23 +499,23 @@ HTMLImageElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, // the state gets in Element's attr-setting happen around this // LoadImage call, we could start passing false instead of aNotify // here. - LoadImage(aValue, true, aNotify, eImageLoadType_Normal); + LoadImage(aValue.String(), true, aNotify, eImageLoadType_Normal); mNewRequestsWillNeedAnimationReset = false; } - } else if (aNameSpaceID == kNameSpaceID_None && + } else if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::crossorigin && aNotify) { nsAttrValue attrValue; - ParseCORSValue(aValue, attrValue); + ParseCORSValue(aValue.String(), attrValue); if (GetCORSMode() != AttrValueToCORSMode(&attrValue)) { // Force a new load of the image with the new cross origin policy. - forceReload = true; + mForceReload = true; } } else if (aName == nsGkAtoms::referrerpolicy && - aNameSpaceID == kNameSpaceID_None && + aNamespaceID == kNameSpaceID_None && aNotify) { - ReferrerPolicy referrerPolicy = AttributeReferrerPolicyFromString(aValue); + ReferrerPolicy referrerPolicy = AttributeReferrerPolicyFromString(aValue.String()); if (!InResponsiveMode() && referrerPolicy != RP_Unset && referrerPolicy != GetImageReferrerPolicy()) { @@ -554,22 +524,28 @@ HTMLImageElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, // the attribute will neither trigger a reload nor update the referrer // policy of the loading channel (whether it has previously completed or // not). Force a new load of the image with the new referrerpolicy. - forceReload = true; + mForceReload = true; } } - nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, - aValue, aNotify); + return; +} +void +HTMLImageElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + bool aNotify) +{ // Because we load image synchronously in non-responsive-mode, we need to do // reload after the attribute has been set if the reload is triggerred by // cross origin changing. - if (forceReload) { + if (mForceReload) { + mForceReload = false; + if (InResponsiveMode()) { // per spec, full selection runs when this changes, even though // it doesn't directly affect the source selection QueueImageLoadTask(true); - } else { + } else if (OwnerDoc()->IsCurrentActiveDocument()) { // Bug 1076583 - We still use the older synchronous algorithm in // non-responsive mode. Force a new load of the image with the // new cross origin policy @@ -577,7 +553,59 @@ HTMLImageElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, } } - return rv; + return; +} + +nsresult +HTMLImageElement::GetEventTargetParent(EventChainPreVisitor& aVisitor) +{ + // We handle image element with attribute ismap in its corresponding frame + // element. Set mMultipleActionsPrevented here to prevent the click event + // trigger the behaviors in Element::PostHandleEventForLinks + WidgetMouseEvent* mouseEvent = aVisitor.mEvent->AsMouseEvent(); + if (mouseEvent && mouseEvent->IsLeftClickEvent() && IsMap()) { + mouseEvent->mFlags.mMultipleActionsPrevented = true; + } + return nsGenericHTMLElement::GetEventTargetParent(aVisitor); +} + +bool +HTMLImageElement::IsHTMLFocusable(bool aWithMouse, + bool *aIsFocusable, int32_t *aTabIndex) +{ + int32_t tabIndex = TabIndex(); + + if (IsInUncomposedDoc()) { + nsAutoString usemap; + GetUseMap(usemap); + // XXXbz which document should this be using? sXBL/XBL2 issue! I + // think that OwnerDoc() is right, since we don't want to + // assume stuff about the document we're bound to. + if (OwnerDoc()->FindImageMap(usemap)) { + if (aTabIndex) { + // Use tab index on individual map areas + *aTabIndex = (sTabFocusModel & eTabFocus_linksMask)? 0 : -1; + } + // Image map is not focusable itself, but flag as tabbable + // so that image map areas get walked into. + *aIsFocusable = false; + + return false; + } + } + + if (aTabIndex) { + // Can be in tab order if tabindex >=0 and form controls are tabbable. + *aTabIndex = (sTabFocusModel & eTabFocus_formElementsMask)? tabIndex : -1; + } + + *aIsFocusable = +#ifdef XP_MACOSX + (!aWithMouse || nsFocusManager::sMouseFocusesFormControl) && +#endif + (tabIndex >= 0 || HasAttr(kNameSpaceID_None, nsGkAtoms::tabindex)); + + return false; } nsresult diff --git a/dom/html/HTMLImageElement.h b/dom/html/HTMLImageElement.h index 52480d0a0..bb4a09882 100644 --- a/dom/html/HTMLImageElement.h +++ b/dom/html/HTMLImageElement.h @@ -75,17 +75,6 @@ public: bool IsHTMLFocusable(bool aWithMouse, bool *aIsFocusable, int32_t *aTabIndex) override; - // SetAttr override. C++ is stupid, so have to override both - // overloaded methods. - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent, nsIContent* aBindingParent, bool aCompileEventHandlers) override; @@ -352,6 +341,10 @@ protected: const nsAttrValue* aOldValue, bool aNotify) override; + virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) override; + // This is a weak reference that this element and the HTMLFormElement // cooperate in maintaining. HTMLFormElement* mForm; @@ -365,6 +358,36 @@ private: static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes, nsRuleData* aData); + /** + * This function is called by BeforeSetAttr and OnAttrSetButNotChanged. + * It will not be called if the value is being unset. + * + * @param aNamespaceID the namespace of the attr being set + * @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. + * @param aNotify Whether we plan to notify document observers. + */ + void BeforeMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify); + /** + * This function is called by AfterSetAttr and OnAttrSetButNotChanged. + * It will not be called if the value is being unset. + * + * @param aNamespaceID the namespace of the attr being set + * @param aName the localname of the attribute being set + * @param aNotify Whether we plan to notify document observers. + */ + void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + bool aNotify); + /** + * Used by BeforeMaybeChangeAttr and AfterMaybeChangeAttr to keep track of + * whether a reload needs to be forced after an attribute change that is + * currently in progress. + */ + bool mForceReload; + bool mInDocResponsiveContent; RefPtr mPendingImageLoadTask; diff --git a/dom/html/HTMLLegendElement.cpp b/dom/html/HTMLLegendElement.cpp index 1a593d769..3f329de42 100644 --- a/dom/html/HTMLLegendElement.cpp +++ b/dom/html/HTMLLegendElement.cpp @@ -70,21 +70,6 @@ HTMLLegendElement::GetAttributeChangeHint(const nsIAtom* aAttribute, return retval; } -nsresult -HTMLLegendElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) -{ - return nsGenericHTMLElement::SetAttr(aNameSpaceID, aAttribute, - aPrefix, aValue, aNotify); -} -nsresult -HTMLLegendElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) -{ - return nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute, aNotify); -} - nsresult HTMLLegendElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, nsIContent* aBindingParent, diff --git a/dom/html/HTMLLegendElement.h b/dom/html/HTMLLegendElement.h index e2d71d62d..0b476c119 100644 --- a/dom/html/HTMLLegendElement.h +++ b/dom/html/HTMLLegendElement.h @@ -42,16 +42,6 @@ public: nsAttrValue& aResult) override; virtual nsChangeHint GetAttributeChangeHint(const nsIAtom* aAttribute, int32_t aModType) const override; - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; virtual nsresult Clone(mozilla::dom::NodeInfo* aNodeInfo, nsINode** aResult) const override; diff --git a/dom/html/HTMLMediaElement.cpp b/dom/html/HTMLMediaElement.cpp index 4bcc188d3..05eaf3e8b 100644 --- a/dom/html/HTMLMediaElement.cpp +++ b/dom/html/HTMLMediaElement.cpp @@ -3726,78 +3726,74 @@ int32_t HTMLMediaElement::TabIndexDefault() return 0; } -nsresult HTMLMediaElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) -{ - nsresult rv = - nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, aValue, - aNotify); - if (NS_FAILED(rv)) - return rv; - if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::src) { - DoLoad(); - } - if (aNotify && aNameSpaceID == kNameSpaceID_None) { - if (aName == nsGkAtoms::autoplay) { - StopSuspendingAfterFirstFrame(); - CheckAutoplayDataReady(); - // This attribute can affect AddRemoveSelfReference - AddRemoveSelfReference(); - UpdatePreloadAction(); +nsresult +HTMLMediaElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) +{ + if (aNameSpaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::src) { + mSrcMediaSource = nullptr; + if (aValue) { + nsString srcStr = aValue->GetStringValue(); + nsCOMPtr uri; + NewURIFromString(srcStr, getter_AddRefs(uri)); + if (uri && IsMediaSourceURI(uri)) { + nsresult rv = + NS_GetSourceForMediaSourceURI(uri, getter_AddRefs(mSrcMediaSource)); + if (NS_FAILED(rv)) { + nsAutoString spec; + GetCurrentSrc(spec); + const char16_t* params[] = { spec.get() }; + ReportLoadError("MediaLoadInvalidURI", params, ArrayLength(params)); + } + } + } + } else if (aName == nsGkAtoms::autoplay) { + if (aNotify) { + if (aValue) { + StopSuspendingAfterFirstFrame(); + CheckAutoplayDataReady(); + } + // This attribute can affect AddRemoveSelfReference + AddRemoveSelfReference(); + UpdatePreloadAction(); + } } else if (aName == nsGkAtoms::preload) { UpdatePreloadAction(); } } - return rv; + // Since AfterMaybeChangeAttr may call DoLoad, make sure that it is called + // *after* any possible changes to mSrcMediaSource. + if (aValue) { + AfterMaybeChangeAttr(aNameSpaceID, aName, aNotify); + } + + return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, + aValue, aOldValue, aNotify); } -nsresult HTMLMediaElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttr, - bool aNotify) +nsresult +HTMLMediaElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) { - nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttr, aNotify); - if (NS_FAILED(rv)) - return rv; - if (aNotify && aNameSpaceID == kNameSpaceID_None) { - if (aAttr == nsGkAtoms::autoplay) { - // This attribute can affect AddRemoveSelfReference - AddRemoveSelfReference(); - UpdatePreloadAction(); - } else if (aAttr == nsGkAtoms::preload) { - UpdatePreloadAction(); - } - } + AfterMaybeChangeAttr(aNamespaceID, aName, aNotify); - return rv; + return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName, + aValue, aNotify); } -nsresult -HTMLMediaElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, - const nsAttrValue* aOldValue, bool aNotify) +void +HTMLMediaElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + bool aNotify) { - if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::src) { - mSrcMediaSource = nullptr; - if (aValue) { - nsString srcStr = aValue->GetStringValue(); - nsCOMPtr uri; - NewURIFromString(srcStr, getter_AddRefs(uri)); - if (uri && IsMediaSourceURI(uri)) { - nsresult rv = - NS_GetSourceForMediaSourceURI(uri, getter_AddRefs(mSrcMediaSource)); - if (NS_FAILED(rv)) { - nsAutoString spec; - GetCurrentSrc(spec); - const char16_t* params[] = { spec.get() }; - ReportLoadError("MediaLoadInvalidURI", params, ArrayLength(params)); - } - } + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::src) { + DoLoad(); } } - - return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, - aValue, aOldValue, aNotify); } nsresult HTMLMediaElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, diff --git a/dom/html/HTMLMediaElement.h b/dom/html/HTMLMediaElement.h index d81e9f591..0d8d5d4e0 100644 --- a/dom/html/HTMLMediaElement.h +++ b/dom/html/HTMLMediaElement.h @@ -132,22 +132,6 @@ public: nsIAtom* aAttribute, const nsAString& aValue, nsAttrValue& aResult) override; - // SetAttr override. C++ is stupid, so have to override both - // overloaded methods. - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttr, - bool aNotify) override; - virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, - const nsAttrValue* aOldValue, - bool aNotify) override; virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent, nsIContent* aBindingParent, @@ -1315,6 +1299,14 @@ protected: already_AddRefed CreateDOMPromise(ErrorResult& aRv) const; + virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; + virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) override; + // The current decoder. Load() has been called on this decoder. // At most one of mDecoder and mSrcStream can be non-null. RefPtr mDecoder; @@ -1717,6 +1709,17 @@ private: // We keep track of these because the load algorithm resolves/rejects all // already-dispatched pending play promises. nsTArray mPendingPlayPromisesRunners; + +private: + /** + * This function is called by AfterSetAttr and OnAttrSetButNotChanged. + * It will not be called if the value is being unset. + * + * @param aNamespaceID the namespace of the attr being set + * @param aName the localname of the attribute being set + * @param aNotify Whether we plan to notify document observers. + */ + void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, bool aNotify); }; } // namespace dom diff --git a/dom/html/HTMLObjectElement.cpp b/dom/html/HTMLObjectElement.cpp index 65f407889..cd2f3f37a 100644 --- a/dom/html/HTMLObjectElement.cpp +++ b/dom/html/HTMLObjectElement.cpp @@ -298,41 +298,45 @@ HTMLObjectElement::UnbindFromTree(bool aDeep, nsresult -HTMLObjectElement::SetAttr(int32_t aNameSpaceID, nsIAtom *aName, - nsIAtom *aPrefix, const nsAString &aValue, - bool aNotify) +HTMLObjectElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { - nsresult rv = nsGenericHTMLFormElement::SetAttr(aNameSpaceID, aName, aPrefix, - aValue, aNotify); + nsresult rv = AfterMaybeChangeAttr(aNamespaceID, aName, aNotify); NS_ENSURE_SUCCESS(rv, rv); - // if aNotify is false, we are coming from the parser or some such place; - // we'll get bound after all the attributes have been set, so we'll do the - // object load from BindToTree/DoneAddingChildren. - // Skip the LoadObject call in that case. - // We also don't want to start loading the object when we're not yet in - // a document, just in case that the caller wants to set additional - // attributes before inserting the node into the document. - if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren && - aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::data) { - return LoadObject(aNotify, true); - } - - return NS_OK; + return nsGenericHTMLFormElement::AfterSetAttr(aNamespaceID, aName, aValue, + aOldValue, aNotify); } nsresult -HTMLObjectElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) +HTMLObjectElement::OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) { - nsresult rv = nsGenericHTMLFormElement::UnsetAttr(aNameSpaceID, - aAttribute, aNotify); + nsresult rv = AfterMaybeChangeAttr(aNamespaceID, aName, aNotify); NS_ENSURE_SUCCESS(rv, rv); - // See comment in SetAttr - if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren && - aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::data) { - return LoadObject(aNotify, true); + return nsGenericHTMLFormElement::OnAttrSetButNotChanged(aNamespaceID, aName, + aValue, aNotify); +} + +nsresult +HTMLObjectElement::AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + bool aNotify) +{ + if (aNamespaceID == kNameSpaceID_None) { + // if aNotify is false, we are coming from the parser or some such place; + // we'll get bound after all the attributes have been set, so we'll do the + // object load from BindToTree/DoneAddingChildren. + // Skip the LoadObject call in that case. + // We also don't want to start loading the object when we're not yet in + // a document, just in case that the caller wants to set additional + // attributes before inserting the node into the document. + if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren && + aName == nsGkAtoms::data) { + return LoadObject(aNotify, true); + } } return NS_OK; diff --git a/dom/html/HTMLObjectElement.h b/dom/html/HTMLObjectElement.h index 5226154da..6ce09e1cc 100644 --- a/dom/html/HTMLObjectElement.h +++ b/dom/html/HTMLObjectElement.h @@ -59,11 +59,6 @@ public: bool aCompileEventHandlers) override; virtual void UnbindFromTree(bool aDeep = true, bool aNullParent = true) override; - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom *aName, - nsIAtom *aPrefix, const nsAString &aValue, - bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; virtual bool IsHTMLFocusable(bool aWithMouse, bool *aIsFocusable, int32_t *aTabIndex) override; virtual IMEState GetDesiredIMEState() override; @@ -245,6 +240,15 @@ public: return GetContentDocument(aSubjectPrincipal); } + protected: + virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; + virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) override; + private: /** * Calls LoadObject with the correct arguments to start the plugin load. @@ -269,6 +273,18 @@ private: static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes, nsRuleData* aData); + /** + * This function is called by AfterSetAttr and OnAttrSetButNotChanged. + * This function will be called by AfterSetAttr whether the attribute is being + * set or unset. + * + * @param aNamespaceID the namespace of the attr being set + * @param aName the localname of the attribute being set + * @param aNotify Whether we plan to notify document observers. + */ + nsresult AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + bool aNotify); + bool mIsDoneAddingChildren; }; diff --git a/dom/html/HTMLSelectElement.cpp b/dom/html/HTMLSelectElement.cpp index d11f8b90f..36dac0852 100644 --- a/dom/html/HTMLSelectElement.cpp +++ b/dom/html/HTMLSelectElement.cpp @@ -1272,9 +1272,22 @@ HTMLSelectElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValueOrString* aValue, bool aNotify) { - if (aNotify && aName == nsGkAtoms::disabled && - aNameSpaceID == kNameSpaceID_None) { - mDisabledChanged = true; + if (aNameSpaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::disabled) { + if (aNotify) { + mDisabledChanged = true; + } + } else if (aName == nsGkAtoms::multiple) { + if (!aValue && aNotify && mSelectedIndex >= 0) { + // We're changing from being a multi-select to a single-select. + // Make sure we only have one option selected before we do that. + // Note that this needs to come before we really unset the attr, + // since SetOptionsSelectedByIndex does some bail-out type + // optimization for cases when the select is not multiple that + // would lead to only a single option getting deselected. + SetSelectedIndexInternal(mSelectedIndex, aNotify); + } + } } return nsGenericHTMLFormElementWithState::BeforeSetAttr(aNameSpaceID, aName, @@ -1294,6 +1307,12 @@ HTMLSelectElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } else if (aName == nsGkAtoms::autocomplete) { // Clear the cached @autocomplete attribute state mAutocompleteAttrState = nsContentUtils::eAutocompleteAttrState_Unknown; + } else if (aName == nsGkAtoms::multiple) { + if (!aValue && aNotify) { + // We might have become a combobox; make sure _something_ gets + // selected in that case + CheckSelectSomething(aNotify); + } } } @@ -1302,37 +1321,6 @@ HTMLSelectElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, aNotify); } -nsresult -HTMLSelectElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) -{ - if (aNotify && aNameSpaceID == kNameSpaceID_None && - aAttribute == nsGkAtoms::multiple) { - // We're changing from being a multi-select to a single-select. - // Make sure we only have one option selected before we do that. - // Note that this needs to come before we really unset the attr, - // since SetOptionsSelectedByIndex does some bail-out type - // optimization for cases when the select is not multiple that - // would lead to only a single option getting deselected. - if (mSelectedIndex >= 0) { - SetSelectedIndexInternal(mSelectedIndex, aNotify); - } - } - - nsresult rv = nsGenericHTMLFormElementWithState::UnsetAttr(aNameSpaceID, aAttribute, - aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - if (aNotify && aNameSpaceID == kNameSpaceID_None && - aAttribute == nsGkAtoms::multiple) { - // We might have become a combobox; make sure _something_ gets - // selected in that case - CheckSelectSomething(aNotify); - } - - return rv; -} - void HTMLSelectElement::DoneAddingChildren(bool aHaveNotified) { diff --git a/dom/html/HTMLSelectElement.h b/dom/html/HTMLSelectElement.h index 90e13da7a..298b1a8d1 100644 --- a/dom/html/HTMLSelectElement.h +++ b/dom/html/HTMLSelectElement.h @@ -381,8 +381,6 @@ public: const nsAttrValue* aValue, const nsAttrValue* aOldValue, bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; virtual void DoneAddingChildren(bool aHaveNotified) override; virtual bool IsDoneAddingChildren() override { diff --git a/dom/html/HTMLSharedElement.cpp b/dom/html/HTMLSharedElement.cpp index bcb84d29b..a7a10b082 100644 --- a/dom/html/HTMLSharedElement.cpp +++ b/dom/html/HTMLSharedElement.cpp @@ -231,52 +231,32 @@ SetBaseTargetUsingFirstBaseWithTarget(nsIDocument* aDocument, } nsresult -HTMLSharedElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) +HTMLSharedElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { - nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, - aValue, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - // If the href attribute of a tag is changing, we may need to update - // the document's base URI, which will cause all the links on the page to be - // re-resolved given the new base. If the target attribute is changing, we - // similarly need to change the base target. - if (mNodeInfo->Equals(nsGkAtoms::base) && - aNameSpaceID == kNameSpaceID_None && - IsInUncomposedDoc()) { - if (aName == nsGkAtoms::href) { - SetBaseURIUsingFirstBaseWithHref(GetUncomposedDoc(), this); - } else if (aName == nsGkAtoms::target) { - SetBaseTargetUsingFirstBaseWithTarget(GetUncomposedDoc(), this); - } - } - - return NS_OK; -} - -nsresult -HTMLSharedElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, - bool aNotify) -{ - nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aName, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - // If we're the first with an href and our href attribute is being - // unset, then we're no longer the first with an href, and we need to - // find the new one. Similar for target. - if (mNodeInfo->Equals(nsGkAtoms::base) && - aNameSpaceID == kNameSpaceID_None && - IsInUncomposedDoc()) { + if (aNamespaceID == kNameSpaceID_None) { if (aName == nsGkAtoms::href) { - SetBaseURIUsingFirstBaseWithHref(GetUncomposedDoc(), nullptr); + // If the href attribute of a tag is changing, we may need to + // update the document's base URI, which will cause all the links on the + // page to be re-resolved given the new base. + // If the href is being unset (aValue is null), we will need to find a new + // . + if (mNodeInfo->Equals(nsGkAtoms::base) && IsInUncomposedDoc()) { + SetBaseURIUsingFirstBaseWithHref(GetUncomposedDoc(), + aValue ? this : nullptr); + } } else if (aName == nsGkAtoms::target) { - SetBaseTargetUsingFirstBaseWithTarget(GetUncomposedDoc(), nullptr); + // The target attribute is in pretty much the same situation as the href + // attribute, above. + if (mNodeInfo->Equals(nsGkAtoms::base) && IsInUncomposedDoc()) { + SetBaseTargetUsingFirstBaseWithTarget(GetUncomposedDoc(), + aValue ? this : nullptr); + } } } - - return NS_OK; + return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue, + aOldValue, aNotify); } nsresult diff --git a/dom/html/HTMLSharedElement.h b/dom/html/HTMLSharedElement.h index 205750cf6..a12ee096d 100644 --- a/dom/html/HTMLSharedElement.h +++ b/dom/html/HTMLSharedElement.h @@ -59,17 +59,6 @@ public: nsIAtom* aAttribute, const nsAString& aValue, nsAttrValue& aResult) override; - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, - bool aNotify) override; virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent, nsIContent* aBindingParent, @@ -181,6 +170,11 @@ protected: virtual ~HTMLSharedElement(); virtual JSObject* WrapNode(JSContext* aCx, JS::Handle aGivenProto) override; + + virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; }; } // namespace dom diff --git a/dom/html/HTMLSharedObjectElement.cpp b/dom/html/HTMLSharedObjectElement.cpp index 889fd5aef..4f35068e1 100644 --- a/dom/html/HTMLSharedObjectElement.cpp +++ b/dom/html/HTMLSharedObjectElement.cpp @@ -161,27 +161,53 @@ HTMLSharedObjectElement::UnbindFromTree(bool aDeep, nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent); } +nsresult +HTMLSharedObjectElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) +{ + if (aValue) { + nsresult rv = AfterMaybeChangeAttr(aNamespaceID, aName, aNotify); + NS_ENSURE_SUCCESS(rv, rv); + } + + return nsGenericHTMLElement::AfterSetAttr(aNamespaceID, aName, aValue, + aOldValue, aNotify); +} nsresult -HTMLSharedObjectElement::SetAttr(int32_t aNameSpaceID, nsIAtom *aName, - nsIAtom *aPrefix, const nsAString &aValue, - bool aNotify) +HTMLSharedObjectElement::OnAttrSetButNotChanged(int32_t aNamespaceID, + nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) { - nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, - aValue, aNotify); + nsresult rv = AfterMaybeChangeAttr(aNamespaceID, aName, aNotify); NS_ENSURE_SUCCESS(rv, rv); - // if aNotify is false, we are coming from the parser or some such place; - // we'll get bound after all the attributes have been set, so we'll do the - // object load from BindToTree/DoneAddingChildren. - // Skip the LoadObject call in that case. - // We also don't want to start loading the object when we're not yet in - // a document, just in case that the caller wants to set additional - // attributes before inserting the node into the document. - if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren && - aNameSpaceID == kNameSpaceID_None && aName == URIAttrName() - && !BlockEmbedContentLoading()) { - return LoadObject(aNotify, true); + return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName, + aValue, aNotify); +} + +nsresult +HTMLSharedObjectElement::AfterMaybeChangeAttr(int32_t aNamespaceID, + nsIAtom* aName, + bool aNotify) +{ + if (aNamespaceID == kNameSpaceID_None) { + if (aName == URIAttrName()) { + // If aNotify is false, we are coming from the parser or some such place; + // we'll get bound after all the attributes have been set, so we'll do the + // object load from BindToTree/DoneAddingChildren. + // Skip the LoadObject call in that case. + // We also don't want to start loading the object when we're not yet in + // a document, just in case that the caller wants to set additional + // attributes before inserting the node into the document. + if (aNotify && IsInComposedDoc() && mIsDoneAddingChildren) { + nsresult rv = LoadObject(aNotify, true); + NS_ENSURE_SUCCESS(rv, rv); + } + } } return NS_OK; diff --git a/dom/html/HTMLSharedObjectElement.h b/dom/html/HTMLSharedObjectElement.h index c70ba8ebd..07cf86cb0 100644 --- a/dom/html/HTMLSharedObjectElement.h +++ b/dom/html/HTMLSharedObjectElement.h @@ -57,9 +57,6 @@ public: bool aCompileEventHandlers) override; virtual void UnbindFromTree(bool aDeep = true, bool aNullParent = true) override; - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom *aName, - nsIAtom *aPrefix, const nsAString &aValue, - bool aNotify) override; virtual bool IsHTMLFocusable(bool aWithMouse, bool *aIsFocusable, int32_t *aTabIndex) override; virtual IMEState GetDesiredIMEState() override; @@ -200,6 +197,16 @@ public: * Calls LoadObject with the correct arguments to start the plugin load. */ void StartObjectLoad(bool aNotify, bool aForceLoad); + +protected: + virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; + virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) override; + private: virtual ~HTMLSharedObjectElement(); @@ -221,6 +228,17 @@ private: static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes, nsRuleData* aData); + /** + * This function is called by AfterSetAttr and OnAttrSetButNotChanged. + * It will not be called if the value is being unset. + * + * @param aNamespaceID the namespace of the attr being set + * @param aName the localname of the attribute being set + * @param aNotify Whether we plan to notify document observers. + */ + nsresult AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + bool aNotify); + /** * Decides whether we should load embed node content. * diff --git a/dom/html/nsGenericHTMLElement.cpp b/dom/html/nsGenericHTMLElement.cpp index 569dae6c2..be78dc1cf 100644 --- a/dom/html/nsGenericHTMLElement.cpp +++ b/dom/html/nsGenericHTMLElement.cpp @@ -654,6 +654,43 @@ nsGenericHTMLElement::GetHrefURIForAnchors() const return uri.forget(); } +nsresult +nsGenericHTMLElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) +{ + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::accesskey) { + // Have to unregister before clearing flag. See UnregAccessKey + UnregAccessKey(); + if (!aValue) { + UnsetFlags(NODE_HAS_ACCESSKEY); + } + } else if (aName == nsGkAtoms::name) { + // Have to do this before clearing flag. See RemoveFromNameTable + RemoveFromNameTable(); + if (!aValue || aValue->IsEmpty()) { + ClearHasName(); + } + } else if (aName == nsGkAtoms::contenteditable) { + if (aValue) { + // Set this before the attribute is set so that any subclass code that + // runs before the attribute is set won't think we're missing a + // contenteditable attr when we actually have one. + SetMayHaveContentEditableAttr(); + } + } + if (!aValue && IsEventAttributeName(aName)) { + if (EventListenerManager* manager = GetExistingListenerManager()) { + manager->RemoveEventHandler(aName, EmptyString()); + } + } + } + + return nsGenericHTMLElementBase::BeforeSetAttr(aNamespaceID, aName, aValue, + aNotify); +} + nsresult nsGenericHTMLElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, const nsAttrValue* aValue, @@ -694,6 +731,34 @@ nsGenericHTMLElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, } } SetDirectionalityOnDescendants(this, dir, aNotify); + } else if (aName == nsGkAtoms::contenteditable) { + int32_t editableCountDelta = 0; + if (aOldValue && + (aOldValue->Equals(NS_LITERAL_STRING("true"), eIgnoreCase) || + aOldValue->Equals(EmptyString(), eIgnoreCase))) { + editableCountDelta = -1; + } + if (aValue && (aValue->Equals(NS_LITERAL_STRING("true"), eIgnoreCase) || + aValue->Equals(EmptyString(), eIgnoreCase))) { + ++editableCountDelta; + } + ChangeEditableState(editableCountDelta); + } else if (aName == nsGkAtoms::accesskey) { + if (aValue && !aValue->Equals(EmptyString(), eIgnoreCase)) { + SetFlags(NODE_HAS_ACCESSKEY); + RegAccessKey(); + } + } else if (aName == nsGkAtoms::name) { + if (aValue && !aValue->Equals(EmptyString(), eIgnoreCase) && + CanHaveName(NodeInfo()->NameAtom())) { + // This may not be quite right because we can have subclass code run + // before here. But in practice subclasses don't care about this flag, + // and in particular selector matching does not care. Otherwise we'd + // want to handle it like we handle id attributes (in PreIdMaybeChange + // and PostIdMaybeChange). + SetHasName(); + AddToNameTable(aValue->GetAtomValue()); + } } } @@ -819,87 +884,6 @@ nsGenericHTMLElement::SetOn##name_(EventHandlerNonNull* handler) \ #undef FORWARDED_EVENT #undef EVENT -nsresult -nsGenericHTMLElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) -{ - bool contentEditable = aNameSpaceID == kNameSpaceID_None && - aName == nsGkAtoms::contenteditable; - bool accessKey = aName == nsGkAtoms::accesskey && - aNameSpaceID == kNameSpaceID_None; - - int32_t change = 0; - if (contentEditable) { - change = GetContentEditableValue() == eTrue ? -1 : 0; - SetMayHaveContentEditableAttr(); - } - - if (accessKey) { - UnregAccessKey(); - } - - nsresult rv = nsStyledElement::SetAttr(aNameSpaceID, aName, aPrefix, aValue, - aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - if (contentEditable) { - if (aValue.IsEmpty() || aValue.LowerCaseEqualsLiteral("true")) { - change += 1; - } - - ChangeEditableState(change); - } - - if (accessKey && !aValue.IsEmpty()) { - SetFlags(NODE_HAS_ACCESSKEY); - RegAccessKey(); - } - - return NS_OK; -} - -nsresult -nsGenericHTMLElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) -{ - bool contentEditable = false; - int32_t contentEditableChange = 0; - - // Check for event handlers - if (aNameSpaceID == kNameSpaceID_None) { - if (aAttribute == nsGkAtoms::name) { - // Have to do this before clearing flag. See RemoveFromNameTable - RemoveFromNameTable(); - ClearHasName(); - } - else if (aAttribute == nsGkAtoms::contenteditable) { - contentEditable = true; - contentEditableChange = GetContentEditableValue() == eTrue ? -1 : 0; - } - else if (aAttribute == nsGkAtoms::accesskey) { - // Have to unregister before clearing flag. See UnregAccessKey - UnregAccessKey(); - UnsetFlags(NODE_HAS_ACCESSKEY); - } - else if (IsEventAttributeName(aAttribute)) { - if (EventListenerManager* manager = GetExistingListenerManager()) { - manager->RemoveEventHandler(aAttribute, EmptyString()); - } - } - } - - nsresult rv = nsGenericHTMLElementBase::UnsetAttr(aNameSpaceID, aAttribute, - aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - if (contentEditable) { - ChangeEditableState(contentEditableChange); - } - - return NS_OK; -} - void nsGenericHTMLElement::GetBaseTarget(nsAString& aBaseTarget) const { @@ -929,19 +913,13 @@ nsGenericHTMLElement::ParseAttribute(int32_t aNamespaceID, if (aAttribute == nsGkAtoms::name) { // Store name as an atom. name="" means that the element has no name, - // not that it has an emptystring as the name. - RemoveFromNameTable(); + // not that it has an empty string as the name. if (aValue.IsEmpty()) { ClearHasName(); return false; } aResult.ParseAtom(aValue); - - if (CanHaveName(NodeInfo()->NameAtom())) { - SetHasName(); - AddToNameTable(aResult.GetAtomValue()); - } return true; } diff --git a/dom/html/nsGenericHTMLElement.h b/dom/html/nsGenericHTMLElement.h index b9c034049..16f438acc 100644 --- a/dom/html/nsGenericHTMLElement.h +++ b/dom/html/nsGenericHTMLElement.h @@ -465,17 +465,6 @@ public: virtual void UnbindFromTree(bool aDeep = true, bool aNullParent = true) override; - MOZ_ALWAYS_INLINE // Avoid a crashy hook from Avast 10 Beta - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, - bool aNotify) override; virtual bool IsFocusableInternal(int32_t *aTabIndex, bool aWithMouse) override { bool isFocusable = false; @@ -914,6 +903,9 @@ private: void RegUnRegAccessKey(bool aDoReg); protected: + virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, const nsAttrValue* aValue, const nsAttrValue* aOldValue, diff --git a/dom/html/nsGenericHTMLFrameElement.cpp b/dom/html/nsGenericHTMLFrameElement.cpp index 1aeb0a7d2..03e6ffb42 100644 --- a/dom/html/nsGenericHTMLFrameElement.cpp +++ b/dom/html/nsGenericHTMLFrameElement.cpp @@ -334,55 +334,6 @@ nsGenericHTMLFrameElement::UnbindFromTree(bool aDeep, bool aNullParent) nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent); } -nsresult -nsGenericHTMLFrameElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) -{ - nsresult rv = nsGenericHTMLElement::SetAttr(aNameSpaceID, aName, aPrefix, - aValue, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::src && - (!IsHTMLElement(nsGkAtoms::iframe) || - !HasAttr(kNameSpaceID_None,nsGkAtoms::srcdoc))) { - // Don't propagate error here. The attribute was successfully set, that's - // what we should reflect. - LoadSrc(); - } else if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::name) { - // Propagate "name" to the docshell to make browsing context names live, - // per HTML5. - nsIDocShell *docShell = mFrameLoader ? mFrameLoader->GetExistingDocShell() - : nullptr; - if (docShell) { - docShell->SetName(aValue); - } - } - - return NS_OK; -} - -nsresult -nsGenericHTMLFrameElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) -{ - // Invoke on the superclass. - nsresult rv = nsGenericHTMLElement::UnsetAttr(aNameSpaceID, aAttribute, aNotify); - NS_ENSURE_SUCCESS(rv, rv); - - if (aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::name) { - // Propagate "name" to the docshell to make browsing context names live, - // per HTML5. - nsIDocShell *docShell = mFrameLoader ? mFrameLoader->GetExistingDocShell() - : nullptr; - if (docShell) { - docShell->SetName(EmptyString()); - } - } - - return NS_OK; -} - /* static */ int32_t nsGenericHTMLFrameElement::MapScrollingAttribute(const nsAttrValue* aValue) { @@ -415,39 +366,88 @@ nsGenericHTMLFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, const nsAttrValue* aOldValue, bool aNotify) { - if (aName == nsGkAtoms::scrolling && aNameSpaceID == kNameSpaceID_None) { - if (mFrameLoader) { - nsIDocShell* docshell = mFrameLoader->GetExistingDocShell(); - nsCOMPtr scrollable = do_QueryInterface(docshell); - if (scrollable) { - int32_t cur; - scrollable->GetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_X, &cur); - int32_t val = MapScrollingAttribute(aValue); - if (cur != val) { - scrollable->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_X, val); - scrollable->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_Y, val); - RefPtr presContext; - docshell->GetPresContext(getter_AddRefs(presContext)); - nsIPresShell* shell = presContext ? presContext->GetPresShell() : nullptr; - nsIFrame* rootScroll = shell ? shell->GetRootScrollFrame() : nullptr; - if (rootScroll) { - shell->FrameNeedsReflow(rootScroll, nsIPresShell::eStyleChange, - NS_FRAME_IS_DIRTY); + if (aValue) { + nsAttrValueOrString value(aValue); + AfterMaybeChangeAttr(aNameSpaceID, aName, &value, aNotify); + } else { + AfterMaybeChangeAttr(aNameSpaceID, aName, nullptr, aNotify); + } + + if (aNameSpaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::scrolling) { + if (mFrameLoader) { + nsIDocShell* docshell = mFrameLoader->GetExistingDocShell(); + nsCOMPtr scrollable = do_QueryInterface(docshell); + if (scrollable) { + int32_t cur; + scrollable->GetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_X, &cur); + int32_t val = MapScrollingAttribute(aValue); + if (cur != val) { + scrollable->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_X, val); + scrollable->SetDefaultScrollbarPreferences(nsIScrollable::ScrollOrientation_Y, val); + RefPtr presContext; + docshell->GetPresContext(getter_AddRefs(presContext)); + nsIPresShell* shell = presContext ? presContext->GetPresShell() : nullptr; + nsIFrame* rootScroll = shell ? shell->GetRootScrollFrame() : nullptr; + if (rootScroll) { + shell->FrameNeedsReflow(rootScroll, nsIPresShell::eStyleChange, + NS_FRAME_IS_DIRTY); + } } } } + } else if (aName == nsGkAtoms::mozbrowser) { + mReallyIsBrowser = !!aValue && BrowserFramesEnabled() && + PrincipalAllowsBrowserFrame(NodePrincipal()); } } - if (aName == nsGkAtoms::mozbrowser && aNameSpaceID == kNameSpaceID_None) { - mReallyIsBrowser = !!aValue && BrowserFramesEnabled() && - PrincipalAllowsBrowserFrame(NodePrincipal()); - } - return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue, aOldValue, aNotify); } +nsresult +nsGenericHTMLFrameElement::OnAttrSetButNotChanged(int32_t aNamespaceID, + nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) +{ + AfterMaybeChangeAttr(aNamespaceID, aName, &aValue, aNotify); + + return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName, + aValue, aNotify); +} + +void +nsGenericHTMLFrameElement::AfterMaybeChangeAttr(int32_t aNamespaceID, + nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) +{ + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::src) { + if (aValue && (!IsHTMLElement(nsGkAtoms::iframe) || + !HasAttr(kNameSpaceID_None, nsGkAtoms::srcdoc))) { + // Don't propagate error here. The attribute was successfully set, + // that's what we should reflect. + LoadSrc(); + } + } else if (aName == nsGkAtoms::name) { + // Propagate "name" to the docshell to make browsing context names live, + // per HTML5. + nsIDocShell* docShell = mFrameLoader ? mFrameLoader->GetExistingDocShell() + : nullptr; + if (docShell) { + if (aValue) { + docShell->SetName(aValue->String()); + } else { + docShell->SetName(EmptyString()); + } + } + } + } +} + void nsGenericHTMLFrameElement::DestroyContent() { diff --git a/dom/html/nsGenericHTMLFrameElement.h b/dom/html/nsGenericHTMLFrameElement.h index 1570b1956..adcd17715 100644 --- a/dom/html/nsGenericHTMLFrameElement.h +++ b/dom/html/nsGenericHTMLFrameElement.h @@ -53,20 +53,7 @@ public: bool aCompileEventHandlers) override; virtual void UnbindFromTree(bool aDeep = true, bool aNullParent = true) override; - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; - virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, - const nsAttrValue* aOldValue, - bool aNotify) override; + virtual void DestroyContent() override; nsresult CopyInnerTo(mozilla::dom::Element* aDest); @@ -111,6 +98,14 @@ protected: nsresult GetContentDocument(nsIDOMDocument** aContentDocument); already_AddRefed GetContentWindow(); + virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; + virtual nsresult OnAttrSetButNotChanged(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString& aValue, + bool aNotify) override; + RefPtr mFrameLoader; nsCOMPtr mOpenerWindow; @@ -133,6 +128,18 @@ protected: private: void GetManifestURL(nsAString& aOut); + + /** + * This function is called by AfterSetAttr and OnAttrSetButNotChanged. + * It will be called whether the value is being set or unset. + * + * @param aNamespaceID the namespace of the attr being set + * @param aName the localname of the attribute being set + * @param aValue the value being set or null if the value is being unset + * @param aNotify Whether we plan to notify document observers. + */ + void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, bool aNotify); }; #endif // nsGenericHTMLFrameElement_h diff --git a/dom/mathml/nsMathMLElement.cpp b/dom/mathml/nsMathMLElement.cpp index e2bc4f008..a74d8168c 100644 --- a/dom/mathml/nsMathMLElement.cpp +++ b/dom/mathml/nsMathMLElement.cpp @@ -1085,50 +1085,27 @@ nsMathMLElement::GetHrefURI() const } nsresult -nsMathMLElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) +nsMathMLElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { - nsresult rv = nsMathMLElementBase::SetAttr(aNameSpaceID, aName, aPrefix, - aValue, aNotify); - - // The ordering of the parent class's SetAttr call and Link::ResetLinkState - // is important here! The attribute is not set until SetAttr returns, and - // we will need the updated attribute value because notifying the document + // It is important that this be done after the attribute is set/unset. + // We will need the updated attribute value because notifying the document // that content states have changed will call IntrinsicState, which will try // to get updated information about the visitedness from Link. if (aName == nsGkAtoms::href && (aNameSpaceID == kNameSpaceID_None || aNameSpaceID == kNameSpaceID_XLink)) { - if (aNameSpaceID == kNameSpaceID_XLink) { + if (aValue && aNameSpaceID == kNameSpaceID_XLink) { WarnDeprecated(u"xlink:href", u"href", OwnerDoc()); } - Link::ResetLinkState(!!aNotify, true); - } - - return rv; -} - -nsresult -nsMathMLElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttr, - bool aNotify) -{ - nsresult rv = nsMathMLElementBase::UnsetAttr(aNameSpaceID, aAttr, aNotify); - - // The ordering of the parent class's UnsetAttr call and Link::ResetLinkState - // is important here! The attribute is not unset until UnsetAttr returns, and - // we will need the updated attribute value because notifying the document - // that content states have changed will call IntrinsicState, which will try - // to get updated information about the visitedness from Link. - if (aAttr == nsGkAtoms::href && - (aNameSpaceID == kNameSpaceID_None || - aNameSpaceID == kNameSpaceID_XLink)) { - // Note: just because we removed a single href attr doesn't mean there's no href, - // since there are 2 possible namespaces. - Link::ResetLinkState(!!aNotify, Link::ElementHasHref()); + // Note: When unsetting href, there may still be another href since there + // are 2 possible namespaces. + Link::ResetLinkState(aNotify, aValue || Link::ElementHasHref()); } - return rv; + return nsMathMLElementBase::AfterSetAttr(aNameSpaceID, aName, aValue, + aOldValue, aNotify); } JSObject* diff --git a/dom/mathml/nsMathMLElement.h b/dom/mathml/nsMathMLElement.h index bbcdb9771..0fdaf021f 100644 --- a/dom/mathml/nsMathMLElement.h +++ b/dom/mathml/nsMathMLElement.h @@ -93,16 +93,6 @@ public: virtual bool IsLink(nsIURI** aURI) const override; virtual void GetLinkTarget(nsAString& aTarget) override; virtual already_AddRefed GetHrefURI() const override; - nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAString& aValue, bool aNotify) - { - return SetAttr(aNameSpaceID, aName, nullptr, aValue, aNotify); - } - virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, - nsIAtom* aPrefix, const nsAString& aValue, - bool aNotify) override; - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; virtual nsIDOMNode* AsDOMNode() override { return this; } @@ -111,6 +101,11 @@ protected: virtual JSObject* WrapNode(JSContext *aCx, JS::Handle aGivenProto) override; + virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; + private: bool mIncrementScriptLevel; }; diff --git a/dom/xbl/XBLChildrenElement.cpp b/dom/xbl/XBLChildrenElement.cpp index e4058a789..f785a3058 100644 --- a/dom/xbl/XBLChildrenElement.cpp +++ b/dom/xbl/XBLChildrenElement.cpp @@ -7,6 +7,7 @@ #include "mozilla/dom/XBLChildrenElement.h" #include "nsCharSeparatedTokenizer.h" #include "mozilla/dom/NodeListBinding.h" +#include "nsAttrValueOrString.h" namespace mozilla { namespace dom { @@ -27,34 +28,24 @@ NS_INTERFACE_MAP_END_INHERITING(Element) NS_IMPL_ELEMENT_CLONE(XBLChildrenElement) nsresult -XBLChildrenElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) +XBLChildrenElement::BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) { - if (aAttribute == nsGkAtoms::includes && - aNameSpaceID == kNameSpaceID_None) { - mIncludes.Clear(); - } - - return Element::UnsetAttr(aNameSpaceID, aAttribute, aNotify); -} - -bool -XBLChildrenElement::ParseAttribute(int32_t aNamespaceID, - nsIAtom* aAttribute, - const nsAString& aValue, - nsAttrValue& aResult) -{ - if (aAttribute == nsGkAtoms::includes && - aNamespaceID == kNameSpaceID_None) { - mIncludes.Clear(); - nsCharSeparatedTokenizer tok(aValue, '|', - nsCharSeparatedTokenizer::SEPARATOR_OPTIONAL); - while (tok.hasMoreTokens()) { - mIncludes.AppendElement(NS_Atomize(tok.nextToken())); + if (aNamespaceID == kNameSpaceID_None) { + if (aName == nsGkAtoms::includes) { + mIncludes.Clear(); + if (aValue) { + nsCharSeparatedTokenizer tok(aValue->String(), '|', + nsCharSeparatedTokenizer::SEPARATOR_OPTIONAL); + while (tok.hasMoreTokens()) { + mIncludes.AppendElement(NS_Atomize(tok.nextToken())); + } + } } } - return false; + return nsXMLElement::BeforeSetAttr(aNamespaceID, aName, aValue, aNotify); } } // namespace dom diff --git a/dom/xbl/XBLChildrenElement.h b/dom/xbl/XBLChildrenElement.h index 4714da4a8..c01085474 100644 --- a/dom/xbl/XBLChildrenElement.h +++ b/dom/xbl/XBLChildrenElement.h @@ -37,14 +37,6 @@ public: virtual nsIDOMNode* AsDOMNode() override { return this; } - // nsIContent interface methods - virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, - bool aNotify) override; - virtual bool ParseAttribute(int32_t aNamespaceID, - nsIAtom* aAttribute, - const nsAString& aValue, - nsAttrValue& aResult) override; - void AppendInsertedChild(nsIContent* aChild) { mInsertedChildren.AppendElement(aChild); @@ -147,6 +139,10 @@ public: protected: ~XBLChildrenElement(); + virtual nsresult BeforeSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValueOrString* aValue, + bool aNotify) override; + private: nsTArray mInsertedChildren; // WEAK nsTArray > mIncludes; -- cgit v1.2.3