Skip to content

Commit 3ae08e3

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 3eddd4c commit 3ae08e3

31 files changed

+287
-2226
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: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9048,8 +9048,9 @@ bool IsLowercaseASCII(const nsAString& aValue) {
90489048
already_AddRefed<Element> Document::CreateElement(
90499049
const nsAString& aTagName, const ElementCreationOptionsOrString& aOptions,
90509050
ErrorResult& rv) {
9051-
rv = nsContentUtils::CheckQName(aTagName, false);
9052-
if (rv.Failed()) {
9051+
// https://dom.spec.whatwg.org/#dom-document-createelement
9052+
if (!nsContentUtils::IsValidElementLocalName(aTagName)) {
9053+
rv.ThrowInvalidCharacterError("Invalid element name");
90539054
return nullptr;
90549055
}
90559056

@@ -9126,8 +9127,9 @@ already_AddRefed<Element> Document::CreateElementNS(
91269127
already_AddRefed<Element> Document::CreateXULElement(
91279128
const nsAString& aTagName, const ElementCreationOptionsOrString& aOptions,
91289129
ErrorResult& aRv) {
9129-
aRv = nsContentUtils::CheckQName(aTagName, false);
9130-
if (aRv.Failed()) {
9130+
// https://dom.spec.whatwg.org/#dom-document-createelement
9131+
if (!nsContentUtils::IsValidElementLocalName(aTagName)) {
9132+
aRv.ThrowInvalidCharacterError("Invalid element name");
91319133
return nullptr;
91329134
}
91339135

@@ -9225,9 +9227,9 @@ already_AddRefed<Attr> Document::CreateAttribute(const nsAString& aName,
92259227
return nullptr;
92269228
}
92279229

9228-
nsresult res = nsContentUtils::CheckQName(aName, false);
9229-
if (NS_FAILED(res)) {
9230-
rv.Throw(res);
9230+
// https://dom.spec.whatwg.org/#dom-document-createattribute
9231+
if (!nsContentUtils::IsValidAttributeLocalName(aName)) {
9232+
rv.ThrowInvalidCharacterError("Invalid attribute name");
92319233
return nullptr;
92329234
}
92339235

@@ -9239,8 +9241,9 @@ already_AddRefed<Attr> Document::CreateAttribute(const nsAString& aName,
92399241
}
92409242

92419243
RefPtr<mozilla::dom::NodeInfo> nodeInfo;
9242-
res = mNodeInfoManager->GetNodeInfo(name, nullptr, kNameSpaceID_None,
9243-
ATTRIBUTE_NODE, getter_AddRefs(nodeInfo));
9244+
nsresult res =
9245+
mNodeInfoManager->GetNodeInfo(name, nullptr, kNameSpaceID_None,
9246+
ATTRIBUTE_NODE, getter_AddRefs(nodeInfo));
92449247
if (NS_FAILED(res)) {
92459248
rv.Throw(res);
92469249
return nullptr;

dom/base/Element.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,8 +1617,9 @@ bool Element::ToggleAttribute(const nsAString& aName,
16171617
const Optional<bool>& aForce,
16181618
nsIPrincipal* aTriggeringPrincipal,
16191619
ErrorResult& aError) {
1620-
aError = nsContentUtils::CheckQName(aName, false);
1621-
if (aError.Failed()) {
1620+
// https://dom.spec.whatwg.org/#dom-element-toggleattribute
1621+
if (!nsContentUtils::IsValidAttributeLocalName(aName)) {
1622+
aError.ThrowInvalidCharacterError("Invalid attribute name");
16221623
return false;
16231624
}
16241625

@@ -1652,8 +1653,9 @@ bool Element::ToggleAttribute(const nsAString& aName,
16521653
void Element::SetAttribute(const nsAString& aName, const nsAString& aValue,
16531654
nsIPrincipal* aTriggeringPrincipal,
16541655
ErrorResult& aError) {
1655-
aError = nsContentUtils::CheckQName(aName, false);
1656-
if (aError.Failed()) {
1656+
// https://dom.spec.whatwg.org/#dom-element-setattribute
1657+
if (!nsContentUtils::IsValidAttributeLocalName(aName)) {
1658+
aError.ThrowInvalidCharacterError("Invalid attribute name");
16571659
return;
16581660
}
16591661

@@ -1777,8 +1779,9 @@ void Element::SetAttribute(
17771779
const nsAString& aName,
17781780
const TrustedHTMLOrTrustedScriptOrTrustedScriptURLOrString& aValue,
17791781
nsIPrincipal* aTriggeringPrincipal, ErrorResult& aError) {
1780-
aError = nsContentUtils::CheckQName(aName, false);
1781-
if (aError.Failed()) {
1782+
// https://dom.spec.whatwg.org/#dom-element-setattribute
1783+
if (!nsContentUtils::IsValidAttributeLocalName(aName)) {
1784+
aError.ThrowInvalidCharacterError("Invalid attribute name");
17821785
return;
17831786
}
17841787

0 commit comments

Comments
 (0)