commit 6705c65b1100b61e5750d1da21cbd9e35a623e06
parent bb9a8fea44ee0ea26f90efaa808e4ea4bc03a9fb
Author: Emilio Cobos Álvarez <emilio@crisal.io>
Date: Thu, 1 Jan 2026 21:15:55 +0000
Bug 2007958 - Use SetParsedAttr for XUL prototype cache. r=smaug
This technically does a bit more work (calling AfterSetAttr), but no
more work than we do for HTML.
I got bit by this while trying to make some XUL attributes (checked /
disabled) work more like their HTML equivalent and their respective
states, because AfterSetAttr doesn't get called otherwise.
Rather than adding more and more special-cases here, I think we should
just take the simplification. I doubt the perf hit is particularly
meaningful nowadays, but I want to land this first to make sure we don't
regress any benchmark.
Declaration blocks don't need to be cloned anymore afaict (but also now
we disallow inline styles by CSP in virtually all the browser chrome).
Differential Revision: https://phabricator.services.mozilla.com/D277624
Diffstat:
3 files changed, 11 insertions(+), 70 deletions(-)
diff --git a/dom/xul/nsXULElement.cpp b/dom/xul/nsXULElement.cpp
@@ -235,26 +235,7 @@ already_AddRefed<Element> nsXULElement::CreateFromPrototype(
}
nsXULElement* element = FromNode(baseElement);
-
- if (aPrototype->mHasIdAttribute) {
- element->SetHasID();
- }
- if (aPrototype->mHasClassAttribute) {
- element->SetMayHaveClass();
- }
- if (aPrototype->mHasStyleAttribute) {
- element->SetMayHaveStyle();
- }
-
element->MakeHeavyweight(aPrototype);
-
- // Check each attribute on the prototype to see if we need to do
- // any additional processing and hookup that would otherwise be
- // done 'automagically' by SetAttr().
- for (const auto& attribute : aPrototype->mAttributes) {
- element->AddListenerForAttributeIfNeeded(attribute.mName);
- }
-
return baseElement.forget();
}
@@ -520,12 +501,6 @@ void nsXULElement::AddListenerForAttributeIfNeeded(nsAtom* aLocalName) {
}
}
-void nsXULElement::AddListenerForAttributeIfNeeded(const nsAttrName& aName) {
- if (aName.IsAtom()) {
- AddListenerForAttributeIfNeeded(aName.Atom());
- }
-}
-
class XULInContentErrorReporter : public Runnable {
public:
explicit XULInContentErrorReporter(Document& aDocument)
@@ -1020,37 +995,12 @@ nsresult nsXULElement::AddPopupListener(nsAtom* aName) {
//----------------------------------------------------------------------
nsresult nsXULElement::MakeHeavyweight(nsXULPrototypeElement* aPrototype) {
- if (!aPrototype) {
- return NS_OK;
- }
-
- size_t i;
- nsresult rv;
- for (i = 0; i < aPrototype->mAttributes.Length(); ++i) {
- nsXULPrototypeAttribute* protoattr = &aPrototype->mAttributes[i];
- nsAttrValue attrValue;
-
- // Style rules need to be cloned.
- if (protoattr->mValue.Type() == nsAttrValue::eCSSDeclaration) {
- DeclarationBlock* decl = protoattr->mValue.GetCSSDeclarationValue();
- RefPtr<DeclarationBlock> declClone = decl->Clone();
-
- nsString stringValue;
- protoattr->mValue.ToString(stringValue);
-
- attrValue.SetTo(declClone.forget(), &stringValue);
- } else {
- attrValue.SetTo(protoattr->mValue);
- }
-
- bool oldValueSet;
- // XXX we might wanna have a SetAndTakeAttr that takes an nsAttrName
- if (protoattr->mName.IsAtom()) {
- rv = SetAndSwapAttr(protoattr->mName.Atom(), attrValue, &oldValueSet);
- } else {
- rv = SetAndSwapAttr(protoattr->mName.NodeInfo(), attrValue, &oldValueSet);
- }
- NS_ENSURE_SUCCESS(rv, rv);
+ MOZ_ASSERT(aPrototype);
+ for (const auto& protoattr : aPrototype->mAttributes) {
+ nsAttrValue value(protoattr.mValue);
+ MOZ_TRY(SetParsedAttr(
+ protoattr.mName.NamespaceID(), protoattr.mName.LocalName(),
+ protoattr.mName.GetPrefix(), value, /* aNotify = */ false));
}
return NS_OK;
}
@@ -1378,7 +1328,6 @@ nsresult nsXULPrototypeElement::SetAttrAt(uint32_t aPos,
}
if (mAttributes[aPos].mName.Equals(nsGkAtoms::id) && !aValue.IsEmpty()) {
- mHasIdAttribute = true;
// Store id as atom.
// id="" means that the element has no id. Not that it has
// emptystring as id.
@@ -1396,13 +1345,11 @@ nsresult nsXULPrototypeElement::SetAttrAt(uint32_t aPos,
return NS_OK;
} else if (mAttributes[aPos].mName.Equals(nsGkAtoms::_class)) {
- mHasClassAttribute = true;
// Compute the element's class list
mAttributes[aPos].mValue.ParseAtomArray(aValue);
return NS_OK;
} else if (mAttributes[aPos].mName.Equals(nsGkAtoms::style)) {
- mHasStyleAttribute = true;
// Parse the element's 'style' attribute
// This is basically duplicating what nsINode::NodePrincipal() does
@@ -1419,6 +1366,7 @@ nsresult nsXULPrototypeElement::SetAttrAt(uint32_t aPos,
aValue, data, eCompatibility_FullStandards, nullptr,
StyleCssRuleType::Style);
if (declaration) {
+ declaration->SetImmutable();
mAttributes[aPos].mValue.SetTo(declaration.forget(), &aValue);
return NS_OK;
diff --git a/dom/xul/nsXULElement.h b/dom/xul/nsXULElement.h
@@ -168,9 +168,6 @@ class nsXULPrototypeElement : public nsXULPrototypeNode {
explicit nsXULPrototypeElement(mozilla::dom::NodeInfo* aNodeInfo = nullptr)
: nsXULPrototypeNode(eType_Element),
mNodeInfo(aNodeInfo),
- mHasIdAttribute(false),
- mHasClassAttribute(false),
- mHasStyleAttribute(false),
mIsAtom(nullptr) {}
private:
@@ -202,9 +199,6 @@ class nsXULPrototypeElement : public nsXULPrototypeNode {
RefPtr<mozilla::dom::NodeInfo> mNodeInfo;
- uint32_t mHasIdAttribute : 1;
- uint32_t mHasClassAttribute : 1;
- uint32_t mHasStyleAttribute : 1;
nsTArray<nsXULPrototypeAttribute> mAttributes; // [OWNER]
RefPtr<nsAtom> mIsAtom;
};
@@ -524,7 +518,6 @@ class nsXULElement : public nsStyledElement {
/**
* Add a listener for the specified attribute, if appropriate.
*/
- void AddListenerForAttributeIfNeeded(const nsAttrName& aName);
void AddListenerForAttributeIfNeeded(nsAtom* aLocalName);
protected:
diff --git a/dom/xul/nsXULPrototypeDocument.cpp b/dom/xul/nsXULPrototypeDocument.cpp
@@ -420,9 +420,6 @@ void nsXULPrototypeDocument::SetIsL10nCached(bool aIsCached) {
void nsXULPrototypeDocument::RebuildPrototypeFromElement(
nsXULPrototypeElement* aPrototype, Element* aElement, bool aDeep) {
- aPrototype->mHasIdAttribute = aElement->HasID();
- aPrototype->mHasClassAttribute = aElement->MayHaveClass();
- aPrototype->mHasStyleAttribute = aElement->MayHaveStyle();
NodeInfo* oldNodeInfo = aElement->NodeInfo();
RefPtr<NodeInfo> newNodeInfo = mNodeInfoManager->GetNodeInfo(
oldNodeInfo->NameAtom(), oldNodeInfo->GetPrefixAtom(),
@@ -448,7 +445,10 @@ void nsXULPrototypeDocument::RebuildPrototypeFromElement(
protoAttr->mName.SetTo(newNodeInfo);
}
protoAttr->mValue.SetTo(*attr.mValue);
-
+ if (protoAttr->mValue.Type() == nsAttrValue::eCSSDeclaration) {
+ // Ensure declarations get copied-on-write if needed.
+ protoAttr->mValue.GetCSSDeclarationValue()->SetImmutable();
+ }
protoAttr++;
}
