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/html/HTMLImageElement.cpp | 162 +++++++++++++++++++++++++----------------- 1 file changed, 95 insertions(+), 67 deletions(-) (limited to 'dom/html/HTMLImageElement.cpp') 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 -- cgit v1.2.3