Bug 1687538 - Make HTMLLinkElement not inherit from Link. r=smaug

As per whatwg/html#6269

Differential Revision: https://phabricator.services.mozilla.com/D103088

dom/base/Link.cpp

@@ -475,12 +475,11 @@ void Link::ResetLinkState(bool aNotify, bool aHasHref) {
     }
   }
 
-  // If we have an href, and we're not a <link>, we should register with the
-  // history.
+  // If we have an href, we should register with the history.
   //
   // FIXME(emilio): Do we really want to allow all MathML elements to be
   // :visited? That seems not great.
-  mNeedsRegistration = aHasHref && !mElement->IsHTMLElement(nsGkAtoms::link);
+  mNeedsRegistration = aHasHref;
 
   // If we've cached the URI, reset always invalidates it.
   UnregisterFromHistory();
@@ -539,11 +538,8 @@ void Link::SetHrefAttribute(nsIURI* aURI) {
 size_t Link::SizeOfExcludingThis(mozilla::SizeOfState& aState) const {
   size_t n = 0;
 
-  if (mCachedURI) {
-    nsCOMPtr<nsISizeOf> iface = do_QueryInterface(mCachedURI);
-    if (iface) {
-      n += iface->SizeOfIncludingThis(aState.mMallocSizeOf);
-    }
+  if (nsCOMPtr<nsISizeOf> iface = do_QueryInterface(mCachedURI)) {
+    n += iface->SizeOfIncludingThis(aState.mMallocSizeOf);
   }
 
   // The following members don't need to be measured:

dom/html/HTMLLinkElement.cpp

@@ -27,6 +27,7 @@
 #include "mozilla/dom/Document.h"
 #include "nsINode.h"
 #include "nsIPrefetchService.h"
+#include "nsISizeOf.h"
 #include "nsPIDOMWindow.h"
 #include "nsReadableUtils.h"
 #include "nsStyleConsts.h"
@@ -47,11 +48,9 @@ namespace mozilla::dom {
 
 HTMLLinkElement::HTMLLinkElement(
     already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo)
-    : nsGenericHTMLElement(std::move(aNodeInfo)), Link(this) {}
+    : nsGenericHTMLElement(std::move(aNodeInfo)) {}
 
-HTMLLinkElement::~HTMLLinkElement() {
-  SupportsDNSPrefetch::Destroyed(*this);
-}
+HTMLLinkElement::~HTMLLinkElement() { SupportsDNSPrefetch::Destroyed(*this); }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(HTMLLinkElement)
 
@@ -69,8 +68,8 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(HTMLLinkElement,
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSizes)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
-NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED(HTMLLinkElement,
-                                             nsGenericHTMLElement, Link)
+NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED_0(HTMLLinkElement,
+                                               nsGenericHTMLElement)
 
 NS_IMPL_ELEMENT_CLONE(HTMLLinkElement)
 
@@ -83,8 +82,6 @@ void HTMLLinkElement::SetDisabled(bool aDisabled, ErrorResult& aRv) {
 }
 
 nsresult HTMLLinkElement::BindToTree(BindContext& aContext, nsINode& aParent) {
-  Link::ResetLinkState(false, Link::ElementHasHref());
-
   nsresult rv = nsGenericHTMLElement::BindToTree(aContext, aParent);
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -117,16 +114,9 @@ void HTMLLinkElement::LinkRemoved() {
 }
 
 void HTMLLinkElement::UnbindFromTree(bool aNullParent) {
-  // Cancel any DNS prefetches
-  // Note: Must come before ResetLinkState.  If called after, it will recreate
-  // mCachedURI based on data that is invalid - due to a call to GetHostname.
   CancelDNSPrefetch(*this);
   CancelPrefetchOrPreload();
 
-  // Without removing the link state we risk a dangling pointer
-  // in the mStyledLinks hashtable
-  Link::ResetLinkState(false, Link::ElementHasHref());
-
   // If this is reinserted back into the document it will not be
   // from the parser.
   Document* oldDoc = GetUncomposedDoc();
@@ -221,23 +211,28 @@ nsresult HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
                                        const nsAttrValue* aOldValue,
                                        nsIPrincipal* aSubjectPrincipal,
                                        bool aNotify) {
-  // It's safe to call ResetLinkState here because our new attr value has
-  // already been set or unset.  ResetLinkState needs 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 && kNameSpaceID_None == aNameSpaceID) {
-    bool hasHref = aValue;
-    Link::ResetLinkState(!!aNotify, hasHref);
+  if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::href) {
+    mCachedURI = nullptr;
     if (IsInUncomposedDoc()) {
       CreateAndDispatchEvent(OwnerDoc(), u"DOMLinkChanged"_ns);
     }
-  }
-
-  if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::href) {
     mTriggeringPrincipal = nsContentUtils::GetAttrTriggeringPrincipal(
         this, aValue ? aValue->GetStringValue() : EmptyString(),
         aSubjectPrincipal);
+
+    // If the link has `rel=localization` and its `href` attribute is changed,
+    // update the list of localization links.
+    if (AttrValueIs(kNameSpaceID_None, nsGkAtoms::rel, nsGkAtoms::localization,
+                    eIgnoreCase)) {
+      if (Document* doc = GetUncomposedDoc()) {
+        if (aOldValue) {
+          doc->LocalizationLinkRemoved(this);
+        }
+        if (aValue) {
+          doc->LocalizationLinkAdded(this);
+        }
+      }
+    }
   }
 
   // If a link's `rel` attribute was changed from or to `localization`,
@@ -257,21 +252,6 @@ nsresult HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
     }
   }
 
-  // If the link has `rel=localization` and its `href` attribute is changed,
-  // update the list of localization links.
-  if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::href &&
-      AttrValueIs(kNameSpaceID_None, nsGkAtoms::rel, nsGkAtoms::localization,
-                  eIgnoreCase)) {
-    if (Document* doc = GetUncomposedDoc()) {
-      if (aOldValue) {
-        doc->LocalizationLinkRemoved(this);
-      }
-      if (aValue) {
-        doc->LocalizationLinkAdded(this);
-      }
-    }
-  }
-
   if (aValue) {
     if (aNameSpaceID == kNameSpaceID_None &&
         (aName == nsGkAtoms::href || aName == nsGkAtoms::rel ||
@@ -330,23 +310,6 @@ nsresult HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
       aNameSpaceID, aName, aValue, aOldValue, aSubjectPrincipal, aNotify);
 }
 
-void HTMLLinkElement::GetEventTargetParent(EventChainPreVisitor& aVisitor) {
-  GetEventTargetParentForAnchors(aVisitor);
-}
-
-nsresult HTMLLinkElement::PostHandleEvent(EventChainPostVisitor& aVisitor) {
-  return PostHandleEventForAnchors(aVisitor);
-}
-
-bool HTMLLinkElement::IsLink(nsIURI** aURI) const { return IsHTMLLink(aURI); }
-
-void HTMLLinkElement::GetLinkTarget(nsAString& aTarget) {
-  GetAttr(kNameSpaceID_None, nsGkAtoms::target, aTarget);
-  if (aTarget.IsEmpty()) {
-    GetBaseTarget(aTarget);
-  }
-}
-
 static const DOMTokenListSupportedToken sSupportedRelValues[] = {
     // Keep this and the one below in sync with ToLinkMask in
     // LinkStyle.cpp.
@@ -380,10 +343,6 @@ nsDOMTokenList* HTMLLinkElement::RelList() {
   return mRelList;
 }
 
-already_AddRefed<nsIURI> HTMLLinkElement::GetHrefURI() const {
-  return GetHrefURIForAnchors();
-}
-
 Maybe<LinkStyle::SheetInfo> HTMLLinkElement::GetStyleSheetInfo() {
   nsAutoString rel;
   GetAttr(kNameSpaceID_None, nsGkAtoms::rel, rel);
@@ -410,16 +369,14 @@ Maybe<LinkStyle::SheetInfo> HTMLLinkElement::GetStyleSheetInfo() {
     return Nothing();
   }
 
-  nsAutoString href;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::href, href);
-  if (href.IsEmpty()) {
+  if (!HasNonEmptyAttr(nsGkAtoms::href)) {
     return Nothing();
   }
 
   nsAutoString integrity;
-  GetAttr(kNameSpaceID_None, nsGkAtoms::integrity, integrity);
+  GetAttr(nsGkAtoms::integrity, integrity);
 
-  nsCOMPtr<nsIURI> uri = Link::GetURI();
+  nsCOMPtr<nsIURI> uri = GetURI();
   nsCOMPtr<nsIPrincipal> prin = mTriggeringPrincipal;
 
   nsAutoString nonce;
@@ -445,14 +402,12 @@ Maybe<LinkStyle::SheetInfo> HTMLLinkElement::GetStyleSheetInfo() {
   });
 }
 
-EventStates HTMLLinkElement::IntrinsicState() const {
-  return Link::LinkState() | nsGenericHTMLElement::IntrinsicState();
-}
-
 void HTMLLinkElement::AddSizeOfExcludingThis(nsWindowSizes& aSizes,
                                              size_t* aNodeSize) const {
   nsGenericHTMLElement::AddSizeOfExcludingThis(aSizes, aNodeSize);
-  *aNodeSize += Link::SizeOfExcludingThis(aSizes.mState);
+  if (nsCOMPtr<nsISizeOf> iface = do_QueryInterface(mCachedURI)) {
+    *aNodeSize += iface->SizeOfExcludingThis(aSizes.mState.mMallocSizeOf);
+  }
 }
 
 JSObject* HTMLLinkElement::WrapNode(JSContext* aCx,
@@ -549,7 +504,7 @@ void HTMLLinkElement::GetContentPolicyMimeTypeMedia(
 void HTMLLinkElement::
     TryDNSPrefetchOrPreconnectOrPrefetchOrPreloadOrPrerender() {
   MOZ_ASSERT(IsInComposedDoc());
-  if (!ElementHasHref()) {
+  if (!HasAttr(nsGkAtoms::href)) {
     return;
   }
 
@@ -568,8 +523,7 @@ void HTMLLinkElement::
     nsCOMPtr<nsIPrefetchService> prefetchService(
         components::Prefetch::Service());
     if (prefetchService) {
-      nsCOMPtr<nsIURI> uri(GetURI());
-      if (uri) {
+      if (nsCOMPtr<nsIURI> uri = GetURI()) {
         auto referrerInfo = MakeRefPtr<ReferrerInfo>(*this);
         prefetchService->PrefetchURI(uri, referrerInfo, this,
                                      linkTypes & ePREFETCH);
@@ -616,7 +570,7 @@ void HTMLLinkElement::UpdatePreload(nsAtom* aName, const nsAttrValue* aValue,
                                     const nsAttrValue* aOldValue) {
   MOZ_ASSERT(IsInComposedDoc());
 
-  if (!ElementHasHref()) {
+  if (!HasAttr(nsGkAtoms::href)) {
     return;
   }
 
@@ -635,7 +589,7 @@ void HTMLLinkElement::UpdatePreload(nsAtom* aName, const nsAttrValue* aValue,
     return;
   }
 
-  nsCOMPtr<nsIURI> uri(GetURI());
+  nsCOMPtr<nsIURI> uri = GetURI();
   if (!uri) {
     return;
   }

dom/html/HTMLLinkElement.h

@@ -22,11 +22,8 @@ class PreloaderBase;
 
 namespace dom {
 
-// NOTE(emilio): If we stop inheriting from Link, we need to remove the
-// IsHTMLElement(nsGkAtoms::link) checks in Link.cpp.
 class HTMLLinkElement final : public nsGenericHTMLElement,
                               public LinkStyle,
-                              public Link,
                               public SupportsDNSPrefetch {
  public:
   explicit HTMLLinkElement(
@@ -45,11 +42,6 @@ class HTMLLinkElement final : public nsGenericHTMLElement,
   void LinkAdded();
   void LinkRemoved();
 
-  // EventTarget
-  void GetEventTargetParent(EventChainPreVisitor& aVisitor) override;
-  MOZ_CAN_RUN_SCRIPT
-  nsresult PostHandleEvent(EventChainPostVisitor& aVisitor) override;
-
   // nsINode
   nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override;
   JSObject* WrapNode(JSContext* aCx,
@@ -64,23 +56,25 @@ class HTMLLinkElement final : public nsGenericHTMLElement,
   nsresult AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
                         const nsAttrValue* aValue, const nsAttrValue* aOldValue,
                         nsIPrincipal* aSubjectPrincipal, bool aNotify) override;
-  bool IsLink(nsIURI** aURI) const override;
-  already_AddRefed<nsIURI> GetHrefURI() const override;
-
   // Element
   bool ParseAttribute(int32_t aNamespaceID, nsAtom* aAttribute,
                       const nsAString& aValue,
                       nsIPrincipal* aMaybeScriptedPrincipal,
                       nsAttrValue& aResult) override;
-  void GetLinkTarget(nsAString& aTarget) override;
-  EventStates IntrinsicState() const override;
 
   void CreateAndDispatchEvent(Document* aDoc, const nsAString& aEventName);
 
   // WebIDL
   bool Disabled() const;
   void SetDisabled(bool aDisabled, ErrorResult& aRv);
 
+  nsIURI* GetURI() {
+    if (!mCachedURI) {
+      GetURIAttr(nsGkAtoms::href, nullptr, getter_AddRefs(mCachedURI));
+    }
+    return mCachedURI.get();
+  }
+
   void GetHref(nsAString& aValue) {
     GetURIAttr(nsGkAtoms::href, nullptr, aValue);
   }
@@ -178,7 +172,7 @@ class HTMLLinkElement final : public nsGenericHTMLElement,
   }
 
   void NodeInfoChanged(Document* aOldDoc) final {
-    ClearHasPendingLinkUpdate();
+    mCachedURI = nullptr;
     nsGenericHTMLElement::NodeInfoChanged(aOldDoc);
   }
 
@@ -218,6 +212,9 @@ class HTMLLinkElement final : public nsGenericHTMLElement,
   // the preload is held alive by other means.
   WeakPtr<PreloaderBase> mPreload;
 
+  // The cached href attribute value.
+  nsCOMPtr<nsIURI> mCachedURI;
+
   // The "explicitly enabled" flag. This flag is set whenever the `disabled`
   // attribute is explicitly unset, and makes alternate stylesheets not be
   // disabled by default anymore.

dom/html/test/test_bug372098.html

@@ -15,7 +15,6 @@
   <div id="content" style="display:none;">
     <iframe name="bug372098"></iframe>
     <a id="a" href="bug372098-link-target.html?a" target="">link</a>
-    <link id="link" href="bug372098-link-target.html?link" target=""/>
     <map>
       <area id="area" shape="default" href="bug372098-link-target.html?area" target=""/>
     </map>
@@ -24,7 +23,6 @@
     <script class="testbody" type="text/javascript">
 
 var a_passed = false;
-var link_passed = false;
 var area_passed = false;
 
 /* Start the test */
@@ -42,7 +40,6 @@
 function finish_test()
 {
   ok(a_passed, "The 'a' element used the correct target.");
-  ok(link_passed, "The 'link' element used the correct target.");
   ok(area_passed, "The 'area' element used the correct target.");
   SimpleTest.finish();
 }
@@ -54,10 +51,6 @@
   switch (tag) {
   case 'a':
     a_passed = true;
-    sendMouseEvent({type:'click'}, 'link');
-    return;
-  case 'link':
-    link_passed = true;
     sendMouseEvent({type:'click'}, 'area');
     return;
   case 'area':

layout/style/test/moz.build

@@ -47,8 +47,6 @@ TEST_HARNESS_FILES.testing.mochitest.tests.layout.style.test["css-visited"] += [
     "/layout/reftests/css-visited/color-choice-1.html",
     "/layout/reftests/css-visited/color-on-bullets-1-ref.html",
     "/layout/reftests/css-visited/color-on-bullets-1.html",
-    "/layout/reftests/css-visited/color-on-htmllinkelement-1-ref.html",
-    "/layout/reftests/css-visited/color-on-htmllinkelement-1.html",
     "/layout/reftests/css-visited/color-on-link-1-ref.html",
     "/layout/reftests/css-visited/color-on-link-1.html",
     "/layout/reftests/css-visited/color-on-link-before-1.html",