Skip to content

Commit e149aed

Browse files
sideshowbarkeropettay@mozilla.com
authored andcommitted
Bug 1773312 - Allow more characters in element/attribute names and prefixes r=smaug,dom-core,devtools-reviewers,ochameau
Implement "relaxed" element/attribute name-validation rules, per: - whatwg/dom#1079 - whatwg/html#7991 - whatwg/html#11453 That aligns DOM-API name behavior with HTML parser behavior - which has always allowed a wider range of characters in element and attribute names. New validation functions in nsContentUtils: - IsValidElementLocalName(): For createElement - allows [A-Za-z] start followed by any char except null/whitespace/>//, or [:_>=0x80] start with restricted continuation - IsValidAttributeName(): For setAttribute/toggleAttribute/createAttribute - no null, whitespace, /, >, or = - IsValidNamespacePrefix(): For *NS methods - no null, whitespace, /, or > - IsValidDoctypeName(): For createDocumentType - no null, whitespace, or > - ParseQualifiedNameRelaxed(): Validates and parses qualified names with relaxed rules ParseQualifiedNameRelaxed() correctly implements the "strictly split" algorithm per the DOM spec: for qualified names with multiple colons like "f:o:o", the local name is just the second token ("o"), not everything after the first colon ("o:o"). This matches the spec's requirement to split on all colons and use only splitResult[0] as prefix and splitResult[1] as localName. Deleted outdated DOM Level 1 mochitest tests that tested old XML-based name validation rules; WPT name-validation.html provides coverage for the new relaxed rules. Removed WPT expected-failure .ini files, since all tests now pass. Differential Revision: https://phabricator.services.mozilla.com/D277822
1 parent 621b2d1 commit e149aed

File tree

40 files changed

+299
-3430
lines changed

40 files changed

+299
-3430
lines changed

devtools/client/inspector/markup/test/browser_markup_tag_edit_01.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ var TEST_DATA = [
2525
},
2626
{
2727
desc:
28-
'Try changing an attribute to a quote (") - this should result ' +
29-
"in it being set to an empty string",
28+
'Try changing an attribute to a quote (") - class becomes empty, ' +
29+
"and the trailing quote becomes a valid attribute name",
3030
node: "#node22",
3131
originalAttributes: {
3232
id: "node22",
@@ -37,6 +37,8 @@ var TEST_DATA = [
3737
expectedAttributes: {
3838
id: "node22",
3939
class: "",
40+
// With relaxed attribute name rules, " is now a valid attribute name
41+
'"': "",
4042
},
4143
},
4244
{
@@ -63,6 +65,8 @@ var TEST_DATA = [
6365
expectedAttributes: {
6466
id: "node24",
6567
class: "",
68+
// With relaxed attribute name rules, " is now a valid attribute name
69+
'"': "",
6670
},
6771
},
6872
];

devtools/client/inspector/markup/test/browser_markup_tag_edit_06.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,13 @@ var TEST_DATA = [
2424
},
2525
},
2626
{
27-
desc: "Invalid attribute name",
27+
desc: "Attribute name starting with <",
2828
text: "x='y' <why-would-you-do-this>=\"???\"",
2929
expectedAttributes: {
3030
x: "y",
31+
// HTML parser reads "<why-would-you-do-this" as attr name (> ends tag).
32+
// With relaxed rules, < is now valid in attribute names.
33+
"<why-would-you-do-this": "",
3134
},
3235
},
3336
{

devtools/client/inspector/markup/test/browser_markup_tag_edit_07.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,14 @@ var TEST_DATA = [
4343
{
4444
desc:
4545
'Try add an attribute containing a quote (") attribute by ' +
46-
"clicking the empty space after a node - this should result " +
47-
"in it being set to an empty string",
46+
"clicking the empty space after a node - style becomes empty, " +
47+
"and the trailing quote becomes a valid attribute name",
4848
text: 'class="newclass" style="""',
4949
expectedAttributes: {
5050
class: "newclass",
5151
style: "",
52+
// With relaxed attribute name rules, " is now a valid attribute name
53+
'"': "",
5254
},
5355
},
5456
{

devtools/client/inspector/markup/test/browser_markup_tag_edit_12.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ async function testAttributeDeletion(inspector) {
6363
const attrs = await getAttributesFromEditor("#delattr", inspector);
6464

6565
info("Entering an invalid attribute to delete the attribute");
66-
await editAttributeAndTab('"', inspector);
66+
// Use > which is still forbidden in attribute names with relaxed rules
67+
await editAttributeAndTab(">", inspector);
6768
checkFocusedAttribute(attrs[2], true);
6869

6970
info("Deleting the last attribute");

dom/base/DOMImplementation.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,9 @@ already_AddRefed<DocumentType> DOMImplementation::CreateDocumentType(
5252
return nullptr;
5353
}
5454

55-
aRv = nsContentUtils::CheckQName(aQualifiedName);
56-
if (aRv.Failed()) {
55+
// https://dom.spec.whatwg.org/#dom-domimplementation-createdocumenttype
56+
if (!nsContentUtils::IsValidDoctypeName(aQualifiedName)) {
57+
aRv.ThrowInvalidCharacterError("Invalid doctype name");
5758
return nullptr;
5859
}
5960

@@ -79,7 +80,9 @@ nsresult DOMImplementation::CreateDocument(const nsAString& aNamespaceURI,
7980
if (!aQualifiedName.IsEmpty()) {
8081
const nsString& qName = PromiseFlatString(aQualifiedName);
8182
const char16_t* colon;
82-
rv = nsContentUtils::CheckQName(qName, true, &colon);
83+
// https://dom.spec.whatwg.org/#dom-domimplementation-createdocument
84+
rv = nsContentUtils::ParseQualifiedNameRelaxed(qName, nsINode::ELEMENT_NODE,
85+
&colon);
8386
NS_ENSURE_SUCCESS(rv, rv);
8487

8588
if (colon && (DOMStringIsNull(aNamespaceURI) ||

dom/base/Document.cpp

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8997,8 +8997,9 @@ bool IsLowercaseASCII(const nsAString& aValue) {
89978997
already_AddRefed<Element> Document::CreateElement(
89988998
const nsAString& aTagName, const ElementCreationOptionsOrString& aOptions,
89998999
ErrorResult& rv) {
9000-
rv = nsContentUtils::CheckQName(aTagName, false);
9001-
if (rv.Failed()) {
9000+
// https://dom.spec.whatwg.org/#dom-document-createelement
9001+
if (!nsContentUtils::IsValidElementLocalName(aTagName)) {
9002+
rv.ThrowInvalidCharacterError("Invalid element name");
90029003
return nullptr;
90039004
}
90049005

@@ -9075,8 +9076,9 @@ already_AddRefed<Element> Document::CreateElementNS(
90759076
already_AddRefed<Element> Document::CreateXULElement(
90769077
const nsAString& aTagName, const ElementCreationOptionsOrString& aOptions,
90779078
ErrorResult& aRv) {
9078-
aRv = nsContentUtils::CheckQName(aTagName, false);
9079-
if (aRv.Failed()) {
9079+
// https://dom.spec.whatwg.org/#dom-document-createelement
9080+
if (!nsContentUtils::IsValidElementLocalName(aTagName)) {
9081+
aRv.ThrowInvalidCharacterError("Invalid element name");
90809082
return nullptr;
90819083
}
90829084

@@ -9174,9 +9176,9 @@ already_AddRefed<Attr> Document::CreateAttribute(const nsAString& aName,
91749176
return nullptr;
91759177
}
91769178

9177-
nsresult res = nsContentUtils::CheckQName(aName, false);
9178-
if (NS_FAILED(res)) {
9179-
rv.Throw(res);
9179+
// https://dom.spec.whatwg.org/#dom-document-createattribute
9180+
if (!nsContentUtils::IsValidAttributeLocalName(aName)) {
9181+
rv.ThrowInvalidCharacterError("Invalid attribute name");
91809182
return nullptr;
91819183
}
91829184

@@ -9188,8 +9190,9 @@ already_AddRefed<Attr> Document::CreateAttribute(const nsAString& aName,
91889190
}
91899191

91909192
RefPtr<mozilla::dom::NodeInfo> nodeInfo;
9191-
res = mNodeInfoManager->GetNodeInfo(name, nullptr, kNameSpaceID_None,
9192-
ATTRIBUTE_NODE, getter_AddRefs(nodeInfo));
9193+
nsresult res =
9194+
mNodeInfoManager->GetNodeInfo(name, nullptr, kNameSpaceID_None,
9195+
ATTRIBUTE_NODE, getter_AddRefs(nodeInfo));
91939196
if (NS_FAILED(res)) {
91949197
rv.Throw(res);
91959198
return nullptr;
@@ -11827,20 +11830,21 @@ already_AddRefed<Element> Document::CreateElem(const nsAString& aName,
1182711830
int32_t aNamespaceID,
1182811831
const nsAString* aIs) {
1182911832
#ifdef DEBUG
11830-
nsAutoString qName;
11831-
if (aPrefix) {
11833+
if (!aPrefix) {
11834+
NS_ASSERTION(nsContentUtils::IsValidElementLocalName(aName),
11835+
"Don't pass invalid names to Document::CreateElem");
11836+
} else {
11837+
nsAutoString qName;
1183211838
aPrefix->ToString(qName);
1183311839
qName.Append(':');
11834-
}
11835-
qName.Append(aName);
11840+
qName.Append(aName);
1183611841

11837-
// Note: "a:b:c" is a valid name in non-namespaces XML, and
11838-
// Document::CreateElement can call us with such a name and no prefix,
11839-
// which would cause an error if we just used true here.
11840-
bool nsAware = aPrefix != nullptr || aNamespaceID != GetDefaultNamespaceID();
11841-
NS_ASSERTION(NS_SUCCEEDED(nsContentUtils::CheckQName(qName, nsAware)),
11842-
"Don't pass invalid prefixes to Document::CreateElem, "
11843-
"check caller.");
11842+
const char16_t* localNameEnd;
11843+
const char16_t* colon;
11844+
NS_ASSERTION(NS_SUCCEEDED(nsContentUtils::ParseQualifiedNameRelaxed(
11845+
qName, ELEMENT_NODE, &colon, &localNameEnd)),
11846+
"Don't pass invalid prefixes to Document::CreateElem.");
11847+
}
1184411848
#endif
1184511849

1184611850
RefPtr<mozilla::dom::NodeInfo> nodeInfo;

dom/base/Element.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1678,8 +1678,9 @@ bool Element::ToggleAttribute(const nsAString& aName,
16781678
const Optional<bool>& aForce,
16791679
nsIPrincipal* aTriggeringPrincipal,
16801680
ErrorResult& aError) {
1681-
aError = nsContentUtils::CheckQName(aName, false);
1682-
if (aError.Failed()) {
1681+
// https://dom.spec.whatwg.org/#dom-element-toggleattribute
1682+
if (!nsContentUtils::IsValidAttributeLocalName(aName)) {
1683+
aError.ThrowInvalidCharacterError("Invalid attribute name");
16831684
return false;
16841685
}
16851686

@@ -1713,8 +1714,9 @@ bool Element::ToggleAttribute(const nsAString& aName,
17131714
void Element::SetAttribute(const nsAString& aName, const nsAString& aValue,
17141715
nsIPrincipal* aTriggeringPrincipal,
17151716
ErrorResult& aError) {
1716-
aError = nsContentUtils::CheckQName(aName, false);
1717-
if (aError.Failed()) {
1717+
// https://dom.spec.whatwg.org/#dom-element-setattribute
1718+
if (!nsContentUtils::IsValidAttributeLocalName(aName)) {
1719+
aError.ThrowInvalidCharacterError("Invalid attribute name");
17181720
return;
17191721
}
17201722

@@ -1838,8 +1840,9 @@ void Element::SetAttribute(
18381840
const nsAString& aName,
18391841
const TrustedHTMLOrTrustedScriptOrTrustedScriptURLOrString& aValue,
18401842
nsIPrincipal* aTriggeringPrincipal, ErrorResult& aError) {
1841-
aError = nsContentUtils::CheckQName(aName, false);
1842-
if (aError.Failed()) {
1843+
// https://dom.spec.whatwg.org/#dom-element-setattribute
1844+
if (!nsContentUtils::IsValidAttributeLocalName(aName)) {
1845+
aError.ThrowInvalidCharacterError("Invalid attribute name");
18431846
return;
18441847
}
18451848

0 commit comments

Comments
 (0)