Bug 1762109 - Make the XLink setup a bit saner. r=smaug

Make Link and SVGAElement agree on XLink handling, and make it more
explicit that SVGAElement needs to be a bit more special for SMIL.

Remove dead MathML XLink code.

Differential Revision: https://phabricator.services.mozilla.com/D142546
This commit is contained in:
Emilio Cobos Álvarez 2022-03-31 14:33:57 +00:00
parent 5ef8024075
commit 881f8f938a
10 changed files with 27 additions and 139 deletions

View File

@ -9,6 +9,7 @@
#include "mozilla/EventStates.h"
#include "mozilla/MemoryReporting.h"
#include "mozilla/dom/Element.h"
#include "mozilla/dom/SVGAElement.h"
#include "mozilla/dom/HTMLDNSPrefetch.h"
#include "mozilla/IHistory.h"
#include "mozilla/StaticPrefs_layout.h"
@ -53,9 +54,17 @@ Link::~Link() {
}
bool Link::ElementHasHref() const {
return mElement->HasAttr(kNameSpaceID_None, nsGkAtoms::href) ||
(!mElement->IsHTMLElement() &&
mElement->HasAttr(kNameSpaceID_XLink, nsGkAtoms::href));
if (mElement->HasAttr(nsGkAtoms::href)) {
return true;
}
if (const auto* svg = SVGAElement::FromNode(*mElement)) {
// This can be a HasAttr(kNameSpaceID_XLink, nsGkAtoms::href) check once
// SMIL is fixed to actually mutate DOM attributes rather than faking it.
return svg->HasHref();
}
MOZ_ASSERT(!mElement->IsSVGElement(),
"What other SVG element inherits from Link?");
return false;
}
void Link::VisitedQueryFinished(bool aVisited) {

View File

@ -55,7 +55,6 @@ DEPRECATED_OPERATION(MathML_DeprecatedMencloseNotationRadical)
DEPRECATED_OPERATION(MathML_DeprecatedMfencedElement)
DEPRECATED_OPERATION(MathML_DeprecatedScriptShiftAttributes)
DEPRECATED_OPERATION(MathML_DeprecatedStyleAttribute)
DEPRECATED_OPERATION(MathML_DeprecatedXLinkAttribute)
DEPRECATED_OPERATION(MathML_DeprecatedStixgeneralOperatorStretching)
DEPRECATED_OPERATION(MathML_DeprecatedScriptminsizeAttribute)
DEPRECATED_OPERATION(MathML_DeprecatedScriptsizemultiplierAttribute)

View File

@ -382,8 +382,6 @@ MathML_DeprecatedMfencedElement=MathML element mfenced is deprecated and will be
MathML_DeprecatedScriptShiftAttributes=MathML attributes “subscriptshift” and “superscriptshift” are deprecated and may be removed at a future date.
# LOCALIZATION NOTE: Do not translate MathML, background, color, fontfamily, fontsize, fontstyle and fontweight.
MathML_DeprecatedStyleAttributeWarning=MathML attributes “background”, “color”, “fontfamily”, “fontsize”, “fontstyle” and “fontweight” are deprecated and will be removed at a future date.
# LOCALIZATION NOTE: Do not translate MathML and XLink.
MathML_DeprecatedXLinkAttributeWarning=XLink attributes “href”, “type”, “show” and “actuate” are deprecated on MathML elements and will be removed at a future date.
# LOCALIZATION NOTE: Do not translate MathML and STIXGeneral. %S is a documentation URL.
MathML_DeprecatedStixgeneralOperatorStretchingWarning=Support for rendering stretched MathML operators with STIXGeneral fonts is deprecated and may be removed at a future date. For details about newer fonts that will continue to be supported, see %S
# LOCALIZATION NOTE: Do not translate MathML and scriptminsize.

View File

@ -880,42 +880,6 @@ already_AddRefed<nsIURI> MathMLElement::GetHrefURI() const {
// The REC says: "When user agents encounter MathML elements with both href
// and xlink:href attributes, the href attribute should take precedence."
const nsAttrValue* href = mAttrs.GetAttr(nsGkAtoms::href, kNameSpaceID_None);
if (!href && !StaticPrefs::mathml_xlink_disabled()) {
// To be a clickable XLink for styling and interaction purposes, we require:
//
// xlink:href - must be set
// xlink:type - must be unset or set to "" or set to "simple"
// xlink:show - must be unset or set to "", "new" or "replace"
// xlink:actuate - must be unset or set to "" or "onRequest"
//
// For any other values, we're either not a *clickable* XLink, or the end
// result is poorly specified. Either way, we return false.
static Element::AttrValuesArray sTypeVals[] = {nsGkAtoms::_empty,
nsGkAtoms::simple, nullptr};
static Element::AttrValuesArray sShowVals[] = {
nsGkAtoms::_empty, nsGkAtoms::_new, nsGkAtoms::replace, nullptr};
static Element::AttrValuesArray sActuateVals[] = {
nsGkAtoms::_empty, nsGkAtoms::onRequest, nullptr};
// Optimization: check for href first for early return
href = mAttrs.GetAttr(nsGkAtoms::href, kNameSpaceID_XLink);
if (href &&
FindAttrValueIn(kNameSpaceID_XLink, nsGkAtoms::type, sTypeVals,
eCaseMatters) != Element::ATTR_VALUE_NO_MATCH &&
FindAttrValueIn(kNameSpaceID_XLink, nsGkAtoms::show, sShowVals,
eCaseMatters) != Element::ATTR_VALUE_NO_MATCH &&
FindAttrValueIn(kNameSpaceID_XLink, nsGkAtoms::actuate, sActuateVals,
eCaseMatters) != Element::ATTR_VALUE_NO_MATCH) {
OwnerDoc()->WarnOnceAbout(
dom::DeprecatedOperations::eMathML_DeprecatedXLinkAttribute);
} else {
href = nullptr;
}
}
if (!href) {
return nullptr;
}
@ -928,44 +892,6 @@ already_AddRefed<nsIURI> MathMLElement::GetHrefURI() const {
return hrefURI.forget();
}
void MathMLElement::GetLinkTarget(nsAString& aTarget) {
if (StaticPrefs::mathml_xlink_disabled()) {
MathMLElementBase::GetLinkTarget(aTarget);
return;
}
const nsAttrValue* target =
mAttrs.GetAttr(nsGkAtoms::target, kNameSpaceID_XLink);
if (target) {
OwnerDoc()->WarnOnceAbout(
dom::DeprecatedOperations::eMathML_DeprecatedXLinkAttribute);
target->ToString(aTarget);
}
if (aTarget.IsEmpty()) {
static Element::AttrValuesArray sShowVals[] = {nsGkAtoms::_new,
nsGkAtoms::replace, nullptr};
bool hasDeprecatedShowAttribute = true;
switch (FindAttrValueIn(kNameSpaceID_XLink, nsGkAtoms::show, sShowVals,
eCaseMatters)) {
case ATTR_MISSING:
hasDeprecatedShowAttribute = false;
break;
case 0:
aTarget.AssignLiteral("_blank");
return;
case 1:
return;
}
if (hasDeprecatedShowAttribute) {
OwnerDoc()->WarnOnceAbout(
dom::DeprecatedOperations::eMathML_DeprecatedXLinkAttribute);
}
OwnerDoc()->GetBaseTarget(aTarget);
}
}
bool MathMLElement::IsEventAttributeNameInternal(nsAtom* aName) {
// The intent is to align MathML event attributes on HTML5, so the flag
// EventNameType_HTML is used here.
@ -995,15 +921,7 @@ nsresult MathMLElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
// 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 ||
(!StaticPrefs::mathml_xlink_disabled() &&
aNameSpaceID == kNameSpaceID_XLink))) {
if (aValue && aNameSpaceID == kNameSpaceID_XLink) {
OwnerDoc()->WarnOnceAbout(
dom::DeprecatedOperations::eMathML_DeprecatedXLinkAttribute);
}
// Note: When unsetting href, there may still be another href since there
// are 2 possible namespaces.
if (aName == nsGkAtoms::href && aNameSpaceID == kNameSpaceID_None) {
Link::ResetLinkState(aNotify, aValue || Link::ElementHasHref());
}

View File

@ -61,14 +61,6 @@ already_AddRefed<DOMSVGAnimatedString> SVGAElement::Href() {
: mStringAttributes[XLINK_HREF].ToDOMAnimatedString(this);
}
//----------------------------------------------------------------------
// Link methods
bool SVGAElement::ElementHasHref() const {
return mStringAttributes[HREF].IsExplicitlySet() ||
mStringAttributes[XLINK_HREF].IsExplicitlySet();
}
//----------------------------------------------------------------------
// nsINode methods
@ -231,39 +223,17 @@ bool SVGAElement::IsFocusableInternal(int32_t* aTabIndex, bool aWithMouse) {
return true;
}
bool SVGAElement::HasHref() const {
// Currently our SMIL implementation does not modify the DOM attributes. Once
// we implement the SVG 2 SMIL behaviour this can be removed.
return mStringAttributes[HREF].IsExplicitlySet() ||
mStringAttributes[XLINK_HREF].IsExplicitlySet();
}
already_AddRefed<nsIURI> SVGAElement::GetHrefURI() const {
// To be a clickable XLink for interaction purposes, we require:
//
// xlink:href - must be set
// xlink:type - must be unset or set to "" or set to "simple"
// xlink:show - must be unset or set to "", "new" or "replace"
// xlink:actuate - must be unset or set to "" or "onRequest"
//
// For any other values, we're either not a *clickable* XLink, or the end
// result is poorly specified. Either way, we return null.
//
// FIXME(bug 1762109): This should probably just return the href value to
// match styling. Blink and WebKit seem to just do that.
static Element::AttrValuesArray sTypeVals[] = {nsGkAtoms::_empty,
nsGkAtoms::simple, nullptr};
static Element::AttrValuesArray sShowVals[] = {
nsGkAtoms::_empty, nsGkAtoms::_new, nsGkAtoms::replace, nullptr};
static Element::AttrValuesArray sActuateVals[] = {
nsGkAtoms::_empty, nsGkAtoms::onRequest, nullptr};
// Optimization: check for href first for early return
bool useBareHref = mStringAttributes[HREF].IsExplicitlySet();
if ((useBareHref || mStringAttributes[XLINK_HREF].IsExplicitlySet()) &&
FindAttrValueIn(kNameSpaceID_XLink, nsGkAtoms::type, sTypeVals,
eCaseMatters) != Element::ATTR_VALUE_NO_MATCH &&
FindAttrValueIn(kNameSpaceID_XLink, nsGkAtoms::show, sShowVals,
eCaseMatters) != Element::ATTR_VALUE_NO_MATCH &&
FindAttrValueIn(kNameSpaceID_XLink, nsGkAtoms::actuate, sActuateVals,
eCaseMatters) != Element::ATTR_VALUE_NO_MATCH) {
if (useBareHref || mStringAttributes[XLINK_HREF].IsExplicitlySet()) {
// Get absolute URI
nsAutoString str;
const uint8_t idx = useBareHref ? HREF : XLINK_HREF;

View File

@ -56,6 +56,7 @@ class SVGAElement final : public SVGAElementBase, public Link {
void GetLinkTarget(nsAString& aTarget) override;
already_AddRefed<nsIURI> GetHrefURI() const override;
bool HasHref() const;
virtual EventStates IntrinsicState() const override;
virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
@ -64,9 +65,6 @@ class SVGAElement final : public SVGAElementBase, public Link {
nsIPrincipal* aMaybeScriptedPrincipal,
bool aNotify) override;
// Link
virtual bool ElementHasHref() const override;
// WebIDL
already_AddRefed<DOMSVGAnimatedString> Href();
already_AddRefed<DOMSVGAnimatedString> Target();
@ -91,6 +89,8 @@ class SVGAElement final : public SVGAElementBase, public Link {
SVGAElementBase::NodeInfoChanged(aOldDoc);
}
NS_IMPL_FROMNODE_WITH_TAG(SVGAElement, kNameSpaceID_SVG, nsGkAtoms::a);
protected:
virtual ~SVGAElement() = default;

View File

@ -6,7 +6,7 @@
<body>
<math xmlns="http://www.w3.org/1998/Math/MathML"
xmlns:xlink="http://www.w3.org/1999/xlink">
<mtext xlink:type="simple" xlink:href="www.mozilla.org">MathML Link</mtext>
<mtext xlink:type="simple" href="www.mozilla.org">MathML Link</mtext>
</math>
</body>
</html>

View File

@ -129,7 +129,7 @@ pref(mathml.mfrac_linethickness_names.disabled,false) == mfrac-linethickness-1.x
== mfrac-linethickness-3.xhtml mfrac-linethickness-3-ref.xhtml
pref(mathml.mathspace_names.disabled,false) == mathml-negativespace.html mathml-negativespace-ref.html
== negative-mspace-1.html negative-mspace-1-ref.html
pref(mathml.xlink.disabled,false) != link-1.xhtml link-ref.xhtml
!= link-1.xhtml link-ref.xhtml
== munderover-empty-scripts.html munderover-empty-scripts-ref.html
pref(mathml.mathspace_names.disabled,false) == positive-namedspace.html positive-namedspace-ref.html
== mtable-align-whitespace.html mtable-align-whitespace-ref.html

View File

@ -89,7 +89,7 @@ nsresult SVGAFrame::AttributeChanged(int32_t aNameSpaceID, nsAtom* aAttribute,
aAttribute == nsGkAtoms::href &&
(aNameSpaceID == kNameSpaceID_None ||
aNameSpaceID == kNameSpaceID_XLink)) {
dom::SVGAElement* content = static_cast<dom::SVGAElement*>(GetContent());
auto* content = static_cast<dom::SVGAElement*>(GetContent());
// SMIL may change whether an <a> element is a link, in which case we will
// need to update the link state.

View File

@ -8371,12 +8371,6 @@
value: @IS_NIGHTLY_BUILD@
mirror: always
# Whether to disable support for XLink on MathML elements.
- name: mathml.xlink.disabled
type: bool
value: true
mirror: always
# Whether to disable support for stretching operators with STIXGeneral fonts.
# macos still has the deprecated STIXGeneral font pre-installed.
- name: mathml.stixgeneral_operator_stretching.disabled