Skip to content

Commit 7ef6667

Browse files
annevkannevk
committed
Fix createElement[NS]'s customElements handling
https://bugs.webkit.org/show_bug.cgi?id=288473 rdar://145557562 Reviewed by Ryosuke Niwa. This makes a variety of changes to align ourselves with the latest Scoped Custom Element Registries proposal: 1. Accept DOMString in addition to ElementCreationOptions. This was already part of the DOM standard and is needed for compatibility. (Although to be fair the need has not been proven recently, but now does not seem like the time to find out.) 2. Have createElementNS take the same argument as createElementForBindings. 3. createHTMLElementWithNameValidation no longer needs both TreeScope and Document as they are once again the same. 4. Given that createHTMLElementWithNameValidation defaults registry to document.customElementRegistry it does not seem unlikely that registry is non-null so remove UNLIKELY there. 5. addToScopedCustomElementRegistryMap needs to be called by createElementForBindings and createElementNS as it's relevant for all elements. Since we only need to call it for scoped registries, we don't have to move the registry defaulting to document.customElementRegistry logic to these methods. Tests are upstreamed here: web-platform-tests/wpt#50925 * LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElement.tentative-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElement.tentative.html: * LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElementNS.tentative-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElementNS.tentative.html: Added. * LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/w3c-import.log: * Source/WebCore/dom/Document.cpp: (WebCore::createHTMLElementWithNameValidation): (WebCore::Document::createElementForBindings): (WebCore::Document::createElementNS): * Source/WebCore/dom/Document.h: * Source/WebCore/dom/Document.idl: Canonical link: https://commits.webkit.org/291093@main
1 parent c8726ba commit 7ef6667

File tree

8 files changed

+204
-56
lines changed

8 files changed

+204
-56
lines changed

‎LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElement.tentative-expected.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11

22
PASS createElement should use the global registry by default
33
PASS createElement should use the specified scoped registry
4-
PASS createElement should create a builtin element regardles of a custom element registry specified
4+
PASS createElement should create a builtin element regardless of a custom element registry specified
55
PASS createElement should use the specified global registry
66
PASS createElement should create an upgrade candidate when there is no matching definition in the specified registry
77
PASS createElement should create an upgrade candidate and the candidate should be upgraded when the element is defined
8+
PASS createElement on a non-HTML document should still handle registries correctly
89

‎LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElement.tentative.html

+29-5
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
</head>
99
<body>
1010
<script>
11+
// Keep this ~synchronized with Document-createElementNS
12+
"use strict";
1113

1214
const scopedRegistry = new CustomElementRegistry();
1315
const otherScopedRegistry = new CustomElementRegistry();
@@ -30,12 +32,22 @@
3032
form: HTMLFormElement,
3133
span: HTMLSpanElement,
3234
table: HTMLTableElement,
35+
unknown: HTMLUnknownElement,
3336
};
34-
for (const localName in elements)
35-
assert_true(document.createElement(localName, {customElements: scopedRegistry}) instanceof elements[localName], localName);
36-
for (const localName in elements)
37-
assert_true(document.createElement(localName, {customElements: window.customElements}) instanceof elements[localName], localName);
38-
}, 'createElement should create a builtin element regardles of a custom element registry specified');
37+
for (const localName in elements) {
38+
const scopedElement = document.createElement(localName, {customElements: scopedRegistry});
39+
assert_true(scopedElement instanceof elements[localName], localName);
40+
assert_equals(scopedElement.customElements, scopedRegistry);
41+
42+
const globalExplicitElement = document.createElement(localName, {customElements: window.customElements});
43+
assert_true(globalExplicitElement instanceof elements[localName], localName);
44+
assert_equals(globalExplicitElement.customElements, window.customElements);
45+
46+
const globalImplicitElement = document.createElement(localName);
47+
assert_true(globalImplicitElement instanceof elements[localName], localName);
48+
assert_equals(globalImplicitElement.customElements, window.customElements);
49+
}
50+
}, 'createElement should create a builtin element regardless of a custom element registry specified');
3951

4052
test(() => {
4153
assert_true(document.createElement('a-b', {customElements: window.customElements}) instanceof GlobalABElement);
@@ -59,6 +71,18 @@
5971
assert_true(cdElement instanceof CDElement);
6072
}, 'createElement should create an upgrade candidate and the candidate should be upgraded when the element is defined');
6173

74+
test(() => {
75+
const doc = new Document();
76+
const scopedElement = doc.createElement("time", {customElements: scopedRegistry});
77+
assert_equals(scopedElement.namespaceURI, null);
78+
assert_equals(scopedElement.customElements, scopedRegistry);
79+
80+
const abElement = doc.createElement("a-b", {customElements: scopedRegistry});
81+
assert_equals(abElement.namespaceURI, null);
82+
assert_equals(abElement.customElements, scopedRegistry);
83+
assert_false(abElement instanceof ScopedABElement);
84+
}, 'createElement on a non-HTML document should still handle registries correctly');
85+
6286
</script>
6387
</body>
6488
</html>

‎LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElementNS.tentative-expected.txt

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
2+
PASS createElementNS should use the global registry by default
3+
PASS createElementNS should use the specified scoped registry
4+
PASS createElementNS should create a builtin element regardless of a custom element registry specified
5+
PASS createElementNS should use the specified global registry
6+
PASS createElementNS should create an upgrade candidate when there is no matching definition in the specified registry
7+
PASS createElementNS should create an upgrade candidate and the candidate should be upgraded when the element is defined
8+
PASS createElementNS on a non-HTML document should still handle registries correctly
9+

‎LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElementNS.tentative.html

+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<link rel="help" href="https://github.com/whatwg/html/issues/10854">
5+
<script src="/resources/testharness.js"></script>
6+
<script src="/resources/testharnessreport.js"></script>
7+
</head>
8+
<body>
9+
<script>
10+
// Keep this ~synchronized with Document-createElement
11+
"use strict";
12+
13+
const scopedRegistry = new CustomElementRegistry();
14+
const otherScopedRegistry = new CustomElementRegistry();
15+
class GlobalABElement extends HTMLElement {};
16+
class ScopedABElement extends HTMLElement {};
17+
customElements.define('a-b', GlobalABElement);
18+
scopedRegistry.define('a-b', ScopedABElement);
19+
20+
test(() => {
21+
assert_true(document.createElementNS('http://www.w3.org/1999/xhtml', 'a-b') instanceof GlobalABElement);
22+
}, 'createElementNS should use the global registry by default');
23+
24+
test(() => {
25+
assert_true(document.createElementNS('http://www.w3.org/1999/xhtml', 'a-b', {customElements: scopedRegistry}) instanceof ScopedABElement);
26+
}, 'createElementNS should use the specified scoped registry');
27+
28+
test(() => {
29+
const elements = {
30+
div: HTMLDivElement,
31+
form: HTMLFormElement,
32+
span: HTMLSpanElement,
33+
table: HTMLTableElement,
34+
unknown: HTMLUnknownElement,
35+
};
36+
for (const localName in elements) {
37+
const scopedElement = document.createElementNS('http://www.w3.org/1999/xhtml', localName, {customElements: scopedRegistry});
38+
assert_true(scopedElement instanceof elements[localName], localName);
39+
assert_equals(scopedElement.customElements, scopedRegistry);
40+
41+
const globalExplicitElement = document.createElementNS('http://www.w3.org/1999/xhtml', localName, {customElements: window.customElements});
42+
assert_true(globalExplicitElement instanceof elements[localName], localName);
43+
assert_equals(globalExplicitElement.customElements, window.customElements);
44+
45+
const globalImplicitElement = document.createElementNS('http://www.w3.org/1999/xhtml', localName);
46+
assert_true(globalImplicitElement instanceof elements[localName], localName);
47+
assert_equals(globalImplicitElement.customElements, window.customElements);
48+
}
49+
}, 'createElementNS should create a builtin element regardless of a custom element registry specified');
50+
51+
test(() => {
52+
assert_true(document.createElementNS('http://www.w3.org/1999/xhtml', 'a-b', {customElements: window.customElements}) instanceof GlobalABElement);
53+
}, 'createElementNS should use the specified global registry');
54+
55+
test(() => {
56+
const element = document.createElementNS('http://www.w3.org/1999/xhtml', 'a-b', {customElements: otherScopedRegistry});
57+
assert_equals(element.__proto__.constructor.name, 'HTMLElement');
58+
}, 'createElementNS should create an upgrade candidate when there is no matching definition in the specified registry');
59+
60+
test(() => {
61+
class CDElement extends HTMLElement { }
62+
const registry = new CustomElementRegistry;
63+
const cdElement = document.createElementNS('http://www.w3.org/1999/xhtml', 'c-d', {customElements: registry});
64+
assert_false(cdElement instanceof CDElement);
65+
assert_equals(cdElement.__proto__.constructor.name, 'HTMLElement');
66+
registry.define('c-d', CDElement);
67+
assert_false(cdElement instanceof CDElement); // Not yet upgraded since it's disconnected.
68+
assert_equals(cdElement.__proto__.constructor.name, 'HTMLElement');
69+
document.body.appendChild(cdElement);
70+
assert_true(cdElement instanceof CDElement);
71+
}, 'createElementNS should create an upgrade candidate and the candidate should be upgraded when the element is defined');
72+
73+
test(() => {
74+
const doc = new Document();
75+
const scopedElement = doc.createElementNS(null, "time", {customElements: scopedRegistry});
76+
assert_equals(scopedElement.namespaceURI, null);
77+
assert_equals(scopedElement.customElements, scopedRegistry);
78+
79+
const abElement = doc.createElementNS(null, "a-b", {customElements: scopedRegistry});
80+
assert_equals(abElement.namespaceURI, null);
81+
assert_equals(abElement.customElements, scopedRegistry);
82+
assert_false(abElement instanceof ScopedABElement);
83+
}, 'createElementNS on a non-HTML document should still handle registries correctly');
84+
85+
</script>
86+
</body>
87+
</html>

‎LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/w3c-import.log

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ List of files:
1919
/LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/CustomElementRegistry-initialize.tentative.html
2020
/LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/CustomElementRegistry-upgrade.tentative.html
2121
/LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElement.tentative.html
22+
/LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-createElementNS.tentative.html
2223
/LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Document-importNode.tentative.html
2324
/LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Element-customElements-exceptions.tentative.html
2425
/LayoutTests/imported/w3c/web-platform-tests/custom-elements/revamped-scoped-registry/Element-customElements.tentative.html

‎Source/WebCore/dom/Document.cpp

+72-47
Original file line numberDiff line numberDiff line change
@@ -1489,53 +1489,56 @@ static inline bool isValidHTMLElementName(const QualifiedName& name)
14891489
}
14901490

14911491
template<typename NameType>
1492-
static ExceptionOr<Ref<Element>> createHTMLElementWithNameValidation(TreeScope& treeScope, Document& document, const NameType& name, CustomElementRegistry* registry)
1492+
static ExceptionOr<Ref<Element>> createHTMLElementWithNameValidation(Document& document, const NameType& name, CustomElementRegistry* registry)
14931493
{
1494-
auto result = [&]() -> ExceptionOr<Ref<Element>> {
1495-
RefPtr element = HTMLElementFactory::createKnownElement(name, document);
1496-
if (LIKELY(element))
1497-
return Ref<Element> { element.releaseNonNull() };
1494+
RefPtr element = HTMLElementFactory::createKnownElement(name, document);
1495+
if (LIKELY(element))
1496+
return Ref<Element> { element.releaseNonNull() };
14981497

1499-
if (!registry)
1500-
registry = treeScope.customElementRegistry();
1498+
if (!registry)
1499+
registry = document.customElementRegistry();
15011500

1502-
if (UNLIKELY(registry)) {
1503-
if (RefPtr elementInterface = registry->findInterface(name))
1504-
return elementInterface->constructElementWithFallback(document, *registry, name);
1505-
}
1501+
if (registry) {
1502+
if (RefPtr elementInterface = registry->findInterface(name))
1503+
return elementInterface->constructElementWithFallback(document, *registry, name);
1504+
}
15061505

1507-
if (UNLIKELY(!isValidHTMLElementName(name)))
1508-
return Exception { ExceptionCode::InvalidCharacterError };
1506+
if (UNLIKELY(!isValidHTMLElementName(name)))
1507+
return Exception { ExceptionCode::InvalidCharacterError };
1508+
1509+
return Ref<Element> { createUpgradeCandidateElement(document, registry, name) };
1510+
}
1511+
1512+
ExceptionOr<Ref<Element>> Document::createElementForBindings(const AtomString& name, std::optional<std::variant<String, ElementCreationOptions>>&& argument)
1513+
{
1514+
Ref document = *this;
1515+
RefPtr<CustomElementRegistry> registry;
1516+
if (UNLIKELY(argument)) {
1517+
if (auto* options = std::get_if<ElementCreationOptions>(&*argument))
1518+
registry = options->customElements;
1519+
}
15091520

1510-
return Ref<Element> { createUpgradeCandidateElement(document, registry, name) };
1521+
auto result = [&]() -> ExceptionOr<Ref<Element>> {
1522+
if (document->isHTMLDocument())
1523+
return createHTMLElementWithNameValidation(document, name.convertToASCIILowercase(), registry.get());
1524+
1525+
if (document->isXHTMLDocument())
1526+
return createHTMLElementWithNameValidation(document, name, registry.get());
1527+
1528+
if (!document->isValidName(name))
1529+
return Exception { ExceptionCode::InvalidCharacterError, makeString("Invalid qualified name: '"_s, name, '\'') };
1530+
1531+
return createElement(QualifiedName(nullAtom(), name, nullAtom()), false);
15111532
}();
15121533

1534+
if (result.hasException())
1535+
return result;
15131536
if (UNLIKELY(registry && registry->isScoped())) {
1514-
if (result.hasException())
1515-
return result;
15161537
Ref element = result.releaseReturnValue();
15171538
CustomElementRegistry::addToScopedCustomElementRegistryMap(element, *registry);
15181539
return element;
15191540
}
1520-
15211541
return result;
1522-
1523-
}
1524-
1525-
ExceptionOr<Ref<Element>> Document::createElementForBindings(const AtomString& name, const ElementCreationOptions& options)
1526-
{
1527-
auto& document = documentScope();
1528-
RefPtr registry = options.customElements;
1529-
if (document.isHTMLDocument())
1530-
return createHTMLElementWithNameValidation(*this, document, name.convertToASCIILowercase(), registry.get());
1531-
1532-
if (document.isXHTMLDocument())
1533-
return createHTMLElementWithNameValidation(*this, document, name, registry.get());
1534-
1535-
if (!document.isValidName(name))
1536-
return Exception { ExceptionCode::InvalidCharacterError, makeString("Invalid qualified name: '"_s, name, '\'') };
1537-
1538-
return createElement(QualifiedName(nullAtom(), name, nullAtom()), false);
15391542
}
15401543

15411544
ExceptionOr<Ref<Element>> Document::createElementForBindings(const AtomString& name)
@@ -1909,9 +1912,15 @@ void Document::setActiveCustomElementRegistry(CustomElementRegistry* registry)
19091912
m_activeCustomElementRegistry = registry;
19101913
}
19111914

1912-
ExceptionOr<Ref<Element>> Document::createElementNS(const AtomString& namespaceURI, const AtomString& qualifiedName)
1915+
ExceptionOr<Ref<Element>> Document::createElementNS(const AtomString& namespaceURI, const AtomString& qualifiedName, std::optional<std::variant<String, ElementCreationOptions>>&& argument)
19131916
{
1914-
Ref document = documentScope();
1917+
Ref document = *this;
1918+
RefPtr<CustomElementRegistry> registry;
1919+
if (UNLIKELY(argument)) {
1920+
if (auto* options = std::get_if<ElementCreationOptions>(&*argument))
1921+
registry = options->customElements;
1922+
}
1923+
19151924
auto opportunisticallyMatchedBuiltinElement = ([&]() -> RefPtr<Element> {
19161925
if (namespaceURI == xhtmlNamespaceURI)
19171926
return HTMLElementFactory::createKnownElement(qualifiedName, document, nullptr, /* createdByParser */ false);
@@ -1924,20 +1933,36 @@ ExceptionOr<Ref<Element>> Document::createElementNS(const AtomString& namespaceU
19241933
return nullptr;
19251934
})();
19261935

1927-
if (LIKELY(opportunisticallyMatchedBuiltinElement))
1928-
return opportunisticallyMatchedBuiltinElement.releaseNonNull();
1936+
auto result = [&]() -> ExceptionOr<Ref<Element>> {
1937+
if (LIKELY(opportunisticallyMatchedBuiltinElement))
1938+
return opportunisticallyMatchedBuiltinElement.releaseNonNull();
19291939

1930-
auto parseResult = Document::parseQualifiedName(namespaceURI, qualifiedName);
1931-
if (parseResult.hasException())
1932-
return parseResult.releaseException();
1933-
QualifiedName parsedName { parseResult.releaseReturnValue() };
1934-
if (!Document::hasValidNamespaceForElements(parsedName))
1935-
return Exception { ExceptionCode::NamespaceError };
1940+
auto parseResult = Document::parseQualifiedName(namespaceURI, qualifiedName);
1941+
if (parseResult.hasException())
1942+
return parseResult.releaseException();
1943+
QualifiedName parsedName { parseResult.releaseReturnValue() };
1944+
if (!Document::hasValidNamespaceForElements(parsedName))
1945+
return Exception { ExceptionCode::NamespaceError };
1946+
1947+
if (parsedName.namespaceURI() == xhtmlNamespaceURI)
1948+
return createHTMLElementWithNameValidation(document, parsedName, registry.get());
19361949

1937-
if (parsedName.namespaceURI() == xhtmlNamespaceURI)
1938-
return createHTMLElementWithNameValidation(*this, documentScope(), parsedName, nullptr);
1950+
return createElement(parsedName, false);
1951+
}();
1952+
1953+
if (result.hasException())
1954+
return result;
1955+
if (UNLIKELY(registry && registry->isScoped())) {
1956+
Ref element = result.releaseReturnValue();
1957+
CustomElementRegistry::addToScopedCustomElementRegistryMap(element, *registry);
1958+
return element;
1959+
}
1960+
return result;
1961+
}
19391962

1940-
return createElement(parsedName, false, nullptr);
1963+
ExceptionOr<Ref<Element>> Document::createElementNS(const AtomString& namespaceURI, const AtomString& qualifiedName)
1964+
{
1965+
return createElementNS(namespaceURI, qualifiedName, { });
19411966
}
19421967

19431968
DocumentEventTiming* Document::documentEventTimingFromNavigationTiming()

‎Source/WebCore/dom/Document.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ class Document
509509
void whenVisible(Function<void()>&&);
510510

511511
WEBCORE_EXPORT ExceptionOr<Ref<Element>> createElementForBindings(const AtomString& tagName);
512-
ExceptionOr<Ref<Element>> createElementForBindings(const AtomString& tagName, const ElementCreationOptions&);
512+
ExceptionOr<Ref<Element>> createElementForBindings(const AtomString& tagName, std::optional<std::variant<String, ElementCreationOptions>>&&);
513513
WEBCORE_EXPORT Ref<DocumentFragment> createDocumentFragment();
514514
WEBCORE_EXPORT Ref<Text> createTextNode(String&& data);
515515
WEBCORE_EXPORT Ref<Comment> createComment(String&& data);
@@ -519,6 +519,7 @@ class Document
519519
WEBCORE_EXPORT ExceptionOr<Ref<Attr>> createAttributeNS(const AtomString& namespaceURI, const AtomString& qualifiedName, bool shouldIgnoreNamespaceChecks = false);
520520
WEBCORE_EXPORT ExceptionOr<Ref<Node>> importNode(Node& nodeToImport, std::variant<bool, ImportNodeOptions>&&);
521521
WEBCORE_EXPORT ExceptionOr<Ref<Element>> createElementNS(const AtomString& namespaceURI, const AtomString& qualifiedName);
522+
ExceptionOr<Ref<Element>> createElementNS(const AtomString& namespaceURI, const AtomString& qualifiedName, std::optional<std::variant<String, ElementCreationOptions>>&&);
522523

523524
WEBCORE_EXPORT Ref<Element> createElement(const QualifiedName&, bool createdByParser, CustomElementRegistry* = nullptr);
524525

‎Source/WebCore/dom/Document.idl

+2-2
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ typedef (HTMLScriptElement or SVGScriptElement) HTMLOrSVGScriptElement;
5656
readonly attribute DocumentType? doctype;
5757
[DOMJIT=Getter] readonly attribute Element? documentElement;
5858

59-
[NewObject, ImplementedAs=createElementForBindings] Element createElement([AtomString] DOMString localName, optional ElementCreationOptions options = { });
60-
[NewObject] Element createElementNS([AtomString] DOMString? namespaceURI, [AtomString] DOMString qualifiedName);
59+
[NewObject, ImplementedAs=createElementForBindings] Element createElement([AtomString] DOMString localName, optional (DOMString or ElementCreationOptions) options = {});
60+
[NewObject] Element createElementNS([AtomString] DOMString? namespaceURI, [AtomString] DOMString qualifiedName, optional (DOMString or ElementCreationOptions) options = {});
6161
HTMLCollection getElementsByTagName([AtomString] DOMString qualifiedName);
6262
HTMLCollection getElementsByTagNameNS([AtomString] DOMString? namespaceURI, [AtomString] DOMString localName);
6363
HTMLCollection getElementsByClassName([AtomString] DOMString classNames);

0 commit comments

Comments
 (0)