Allow the creation of null registry element and shadow root [chromium/src : main]
YeongHan Kim (Gerrit)
YeongHan Kim added 1 comment![]( "Open in Gerrit")
Sorry for the delay.
Please review it when you get a chance. Thanks!
Related details
- Jayson Chen
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen voted and added 2 comments![]( "Open in Gerrit")
Votes added by Jayson Chen
| Code-Review | +1 |
2 comments
Overall looks good to me! Thanks for making the change.
const bool wait_for_registry) {Do we still need this argument? (Correct me if I'm wrong) In the default registry scenario (registry not provided to `CreateElement`), if we get null by looking up document's custom element registry by calling `customElementRegistry()`, we do want to set null registry to the element right?
Is it possible to remove this argument, and in line 1632 below use `!registry` for the argument for `CreateUncustomizedOrUndefinedElement`?
Related details
- Mason Freed
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
YeongHan Kim (Gerrit)
YeongHan Kim added 2 comments![]( "Open in Gerrit")
Thank you so much for reviewing this!
Do we still need this argument? (Correct me if I'm wrong) In the default registry scenario (registry not provided to `CreateElement`), if we get null by looking up document's custom element registry by calling `customElementRegistry()`, we do want to set null registry to the element right?
Is it possible to remove this argument, and in line 1632 below use `!registry` for the argument for `CreateUncustomizedOrUndefinedElement`?
Oh! I had thought it needed to be set only when `ElementCreationOptions` included a registry, but it seems I misunderstood.
You’re right, and it looks like we can remove both the `wait_for_registry` argument and the extra `CreateElement()` overload.
Thanks!
Related details
- Jayson Chen
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen voted and added 4 comments![]( "Open in Gerrit")
Votes added by Jayson Chen
| Code-Review | +1 |
4 comments
LGTM with few nits. Thanks!
Element* Document::CreateElement(const QualifiedName& q_name,nit: can we get a comment explaining this overload behavior of "not providing registry in arg (default case) so use document's registry?
// 2. If registry is "default", set registry to the result of looking
// up a custom element registry given document.nit: can we also have these lines in the overload function? Since that is the entry point for "registry is default"
// Note that this step is currently only applicable to scenario when
// scoped registry is disabled as a valid registry should be assigned for
// default cases while flattening options.nit: Prob not entirely accurate now, perhaps something along the line of
"Note that we need to assign default registry when scoped registry is disabled"
to explain the following line of code.
Related details
- Mason Freed
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed voted and added 6 comments![]( "Open in Gerrit")
Votes added by Mason Freed
| Code-Review | +1 |
6 comments
LGTM, thanks for working on this bug! I do have a few nits, but nothing major I think.
Element* CreateElement(const QualifiedName&,
const CreateElementFlags,
const AtomicString& is);I think it might (might!) be better *not* to add this overload, and instead explicitly pass in the registry argument? There are 12 callers or so, which isn't too many. I'm just concerned that all callers really should be thinking about custom element registries, rather than perhaps accidentally relying on the default behavior.
// 3-1. If options["is"] exists, then set is to it.`"is"` - see comment below
"The custom element registry and is option can't be set at the "Perhaps `and "is" option` (with quotation marks) or something? It's too bad "is" is the name of this thing, it makes writing it down confusing.
Element* Document::CreateElement(const QualifiedName& q_name,nit: can we get a comment explaining this overload behavior of "not providing registry in arg (default case) so use document's registry?
...also, I think it'd be easier to read if this overload were defined in the `.h` file instead.
*this, q_name, flags, is, registry, !registry);nit: `, /*wait_for_registry*/ !registry)`
Related details
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen added 1 comment![]( "Open in Gerrit")
Element* CreateElement(const QualifiedName&,
const CreateElementFlags,
const AtomicString& is);I think it might (might!) be better *not* to add this overload, and instead explicitly pass in the registry argument? There are 12 callers or so, which isn't too many. I'm just concerned that all callers really should be thinking about custom element registries, rather than perhaps accidentally relying on the default behavior.
I’d argue that we need the overload for default case bc
- The spec does outline that create element could have a registry given, null or default as registry. Without overloading, we’re not able to differentiate the implicit null (default case) or explicit null (actually want null case) without introducing another argument asking if null is explicit or implicit. I find overloading here clearer and less confusing than having another argument like wait_for_registry. Patch 7 demonstrates what it’s like if we don’t do overload but have another argument.
- Correct me if I’m wrong, it feels like it is actually the desired behavior to use default registry, unless the caller explicitly wants the registry to specified, so I wouldn’t worry too much about relying on the default behavior.
But ofc open to thoughts and discussions :)
YeongHan Kim (Gerrit)
YeongHan Kim added 8 comments![]( "Open in Gerrit")
Thanks for the review!
I’ve applied the nit suggestions, and I’ve also shared some of my thoughts in the discussion.
Element* CreateElement(const QualifiedName&,
const CreateElementFlags,
const AtomicString& is);Jayson ChenI think it might (might!) be better *not* to add this overload, and instead explicitly pass in the registry argument? There are 12 callers or so, which isn't too many. I'm just concerned that all callers really should be thinking about custom element registries, rather than perhaps accidentally relying on the default behavior.
I’d argue that we need the overload for default case bc
- The spec does outline that create element could have a registry given, null or default as registry. Without overloading, we’re not able to differentiate the implicit null (default case) or explicit null (actually want null case) without introducing another argument asking if null is explicit or implicit. I find overloading here clearer and less confusing than having another argument like wait_for_registry. Patch 7 demonstrates what it’s like if we don’t do overload but have another argument.
- Correct me if I’m wrong, it feels like it is actually the desired behavior to use default registry, unless the caller explicitly wants the registry to specified, so I wouldn’t worry too much about relying on the default behavior.
But ofc open to thoughts and discussions :)
Thank you for the helpful thoughts!
I’d like to share my thoughts as well, though cautiously.
When the registry is not explicitly provided (implicit null), we previously passed nullptr, but now it has been changed so that we pass the default (`customElementRegistry()`) instead.
Based on that, the way I understand Mason’s suggestion is that instead of using an overload, we could explicitly pass the default registry. In that case, unlike Patch 7, we wouldn’t need an additional argument
That said, even so, when the registry isn’t explicitly provided, I feel it’s cleaner to rely on the default behavior through an overload rather than requiring callers to explicitly pass the default registry
`"is"` - see comment below
Done!
"The custom element registry and is option can't be set at the "Perhaps `and "is" option` (with quotation marks) or something? It's too bad "is" is the name of this thing, it makes writing it down confusing.
Done!
Element* Document::CreateElement(const QualifiedName& q_name,Mason Freednit: can we get a comment explaining this overload behavior of "not providing registry in arg (default case) so use document's registry?
...also, I think it'd be easier to read if this overload were defined in the `.h` file instead.
I’ve added the comment to the `.h` file. Thanks!
// 2. If registry is "default", set registry to the result of looking
// up a custom element registry given document.nit: can we also have these lines in the overload function? Since that is the entry point for "registry is default"
Done!
// Note that this step is currently only applicable to scenario when
// scoped registry is disabled as a valid registry should be assigned for
// default cases while flattening options.nit: Prob not entirely accurate now, perhaps something along the line of
"Note that we need to assign default registry when scoped registry is disabled"
to explain the following line of code.
Done!
*this, q_name, flags, is, registry, !registry);YeongHan Kimnit: `, /*wait_for_registry*/ !registry)`
Done!
Related details
- Jayson Chen
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Jayson Chen
| Code-Review | +1 |
1 comment
Element* CreateElement(const QualifiedName&,
const CreateElementFlags,
const AtomicString& is);Jayson ChenI think it might (might!) be better *not* to add this overload, and instead explicitly pass in the registry argument? There are 12 callers or so, which isn't too many. I'm just concerned that all callers really should be thinking about custom element registries, rather than perhaps accidentally relying on the default behavior.
YeongHan KimI’d argue that we need the overload for default case bc
- The spec does outline that create element could have a registry given, null or default as registry. Without overloading, we’re not able to differentiate the implicit null (default case) or explicit null (actually want null case) without introducing another argument asking if null is explicit or implicit. I find overloading here clearer and less confusing than having another argument like wait_for_registry. Patch 7 demonstrates what it’s like if we don’t do overload but have another argument.
- Correct me if I’m wrong, it feels like it is actually the desired behavior to use default registry, unless the caller explicitly wants the registry to specified, so I wouldn’t worry too much about relying on the default behavior.
But ofc open to thoughts and discussions :)
Thank you for the helpful thoughts!
I’d like to share my thoughts as well, though cautiously.When the registry is not explicitly provided (implicit null), we previously passed nullptr, but now it has been changed so that we pass the default (`customElementRegistry()`) instead.
Based on that, the way I understand Mason’s suggestion is that instead of using an overload, we could explicitly pass the default registry. In that case, unlike Patch 7, we wouldn’t need an additional argument
That said, even so, when the registry isn’t explicitly provided, I feel it’s cleaner to rely on the default behavior through an overload rather than requiring callers to explicitly pass the default registry
Ah yes that makes sense that we wouldn't need an additional argument then. With that in mind, I still slightly lean towards having the overload function as default registry case is explicitly stated in the spec, but I don't feel as strongly about it as before. I guess the question will come down to, moving forward for all the create element caller who don't care about custom element registry, should we expect them to know that they should pass in `document->customElementRegistry()` as the default value? Or we want to offload that responsibility of knowing registry to the overload function.
Related details
- Mason Freed
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
YeongHan Kim (Gerrit)
YeongHan Kim added 1 comment![]( "Open in Gerrit")
Related details
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed added 1 comment![]( "Open in Gerrit")
Element* CreateElement(const QualifiedName&,
const CreateElementFlags,
const AtomicString& is);Jayson ChenI think it might (might!) be better *not* to add this overload, and instead explicitly pass in the registry argument? There are 12 callers or so, which isn't too many. I'm just concerned that all callers really should be thinking about custom element registries, rather than perhaps accidentally relying on the default behavior.
YeongHan KimI’d argue that we need the overload for default case bc
- The spec does outline that create element could have a registry given, null or default as registry. Without overloading, we’re not able to differentiate the implicit null (default case) or explicit null (actually want null case) without introducing another argument asking if null is explicit or implicit. I find overloading here clearer and less confusing than having another argument like wait_for_registry. Patch 7 demonstrates what it’s like if we don’t do overload but have another argument.
- Correct me if I’m wrong, it feels like it is actually the desired behavior to use default registry, unless the caller explicitly wants the registry to specified, so I wouldn’t worry too much about relying on the default behavior.
But ofc open to thoughts and discussions :)
Jayson ChenThank you for the helpful thoughts!
I’d like to share my thoughts as well, though cautiously.When the registry is not explicitly provided (implicit null), we previously passed nullptr, but now it has been changed so that we pass the default (`customElementRegistry()`) instead.
Based on that, the way I understand Mason’s suggestion is that instead of using an overload, we could explicitly pass the default registry. In that case, unlike Patch 7, we wouldn’t need an additional argument
That said, even so, when the registry isn’t explicitly provided, I feel it’s cleaner to rely on the default behavior through an overload rather than requiring callers to explicitly pass the default registry
Ah yes that makes sense that we wouldn't need an additional argument then. With that in mind, I still slightly lean towards having the overload function as default registry case is explicitly stated in the spec, but I don't feel as strongly about it as before. I guess the question will come down to, moving forward for all the create element caller who don't care about custom element registry, should we expect them to know that they should pass in `document->customElementRegistry()` as the default value? Or we want to offload that responsibility of knowing registry to the overload function.
My concern here is confusion and bugs in future code. Since there are explicitly three choices:
- Implicit null
- Explicit null
- Explicit non-null registry
these three choices need to be apparent in the interface. Right now, there are two overloads, and one of them chooses one of these choices without it being obvious. For example, imagine [this method](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/svg/svg_script_element.cc;l=205;drc=13c2ee3faddbad7baed3819883d2c6438fe01247) were being added *after* this change lands. It might have been possible then, for the author to just not add the fourth argument, and miss that now SVG scripts would not properly construct elements with the right registry. That's bad, right? That relies on tests to catch such bugs, and it relies on the author of the change to know that there's a possible interaction that they might need to add a test for. If nothing breaks when they add it, they'll likely not even notice that there is an interaction.
So based on that, I'm still in favor of requiring the `registry` argument, and I think further we should have a static method on CustomElementRegistry called `CustomElementRegistry::ImplicitNullRegistry()` or similar, which is the one used where that makes sense. That feels easier to understand than just passing `customElementRegistry()`, because "customElementRegistry()" doesn't tell me what this means "implicit registry". I agree with the above that the two argument approach is a hassle. But this method still makes the choice explicit at the call site, rather than implicit.
I'm still open to being talked out of this, so let me know what I'm missing here. I.e. let's discuss before you make a big change to the API again. I want to avoid making you go through the pain if I've got the wrong idea here.
Related details
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
YeongHan Kim (Gerrit)
YeongHan Kim added 1 comment![]( "Open in Gerrit")
Thank you for the detailed explanation!
I agree with your point.
What do you think, Jayson?
Related details
- Jayson Chen
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
YeongHan Kim (Gerrit)
YeongHan Kim added 1 comment![]( "Open in Gerrit")
Related details
- Jayson Chen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
YeongHan Kim (Gerrit)
YeongHan Kim added 2 comments![]( "Open in Gerrit")
Sorry for the repeated review requests. I’ve addressed the feedback now.
Thanks so much!
Element* CreateElement(const QualifiedName&,
const CreateElementFlags,
const AtomicString& is);I removed the overload and explicitly pass the default registry as an argument.
Related details
- Jayson Chen
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen voted and added 3 comments![]( "Open in Gerrit")
Votes added by Jayson Chen
| Code-Review | +1 |
3 comments
Like the direction and LGTM. Just few small nits.
Element* CreateElement(const QualifiedName&,
const CreateElementFlags,
const AtomicString& is);That makes sense to me! I like the idea of having a static method to capture the missing concept here. Instead of `ImplicitNullRegistry()`, I wonder if it makes sense to just call it `DefaultRegistry()` as that will effectively make the argument fall into these three choices:
- Default
- Null
- Non-null registry
which aligns with the spec more; whereas implicit null is more like an implementation detail that is harder to understand while reading the code.
registry = customElementRegistry();If we call the static method `DefaultRegistry()`, I'd suggest that we use the static method here as well.
Related details
- Mason Freed
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
YeongHan Kim (Gerrit)
YeongHan Kim added 4 comments![]( "Open in Gerrit")
Thanks for the review! I’ve addressed your comments.
Element* CreateElement(const QualifiedName&,
const CreateElementFlags,
const AtomicString& is);I agree, `DefaultRegistry()` seems like a more appropriate name.
Thanks for the suggestion!
CustomElementRegistry::DefaultRegistry(*document);I’ve made the same change here to use `DefaultRegistry()`.
If we call the static method `DefaultRegistry()`, I'd suggest that we use the static method here as well.
- Jayson Chen
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen voted Code-Review+1![]( "Open in Gerrit")
- Mason Freed
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed added 5 comments![]( "Open in Gerrit")
This looks good to me, other than a rename/move, and some nits. Thanks for discussing the overloaded function!
"the "Nit: please recombine this with the next line.
/*registry*/ CustomElementRegistry::DefaultRegistry(document)));nit: you don't need `/*registry*/` here since it's clear from the argument that you're passing a registry.
/*registry*/ CustomElementRegistry::DefaultRegistry(GetDocument())));ditto here and below
CustomElementRegistry* CustomElementRegistry::DefaultRegistry(
Document& document) {
return document.customElementRegistry();
}Rather than `CustomElementRegistry::DefaultRegistry(document)`, perhaps just rename `TreeScope::customElementRegistry` to `TreeScope::DefaultCustomElementRegistry` and change all of the callers to this function to just `document.DefaultCustomElementRegistry()`? (And then delete `CustomElementRegistry::DefaultRegistry`.)
Related details
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
YeongHan Kim (Gerrit)
YeongHan Kim added 5 comments![]( "Open in Gerrit")
Thank you for review!
I left a few comments regarding the rename.
"the "YeongHan KimNit: please recombine this with the next line.
Done
/*registry*/ CustomElementRegistry::DefaultRegistry(document)));nit: you don't need `/*registry*/` here since it's clear from the argument that you're passing a registry.
Done
/*registry*/ CustomElementRegistry::DefaultRegistry(GetDocument())));YeongHan Kimditto here and below
Done
CustomElementRegistry* CustomElementRegistry::DefaultRegistry(
Document& document) {
return document.customElementRegistry();
}Rather than `CustomElementRegistry::DefaultRegistry(document)`, perhaps just rename `TreeScope::customElementRegistry` to `TreeScope::DefaultCustomElementRegistry` and change all of the callers to this function to just `document.DefaultCustomElementRegistry()`? (And then delete `CustomElementRegistry::DefaultRegistry`.)
`TreeScope::customElementRegistry()` is used as a mixin in [document_or_shadow_root.idl](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/document_or_shadow_root.idl;l=31), so renaming it would require implementing `customElementRegistry()` on both `Document` and `ShadowRoot`.
Could you please confirm whether this direction is what you intended?
Related details
- Jayson Chen
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed voted and added 2 comments![]( "Open in Gerrit")
Votes added by Mason Freed
| Code-Review | +1 |
2 comments
LGTM! Thanks for working on this.
CustomElementRegistry* CustomElementRegistry::DefaultRegistry(
Document& document) {
return document.customElementRegistry();
}YeongHan KimRather than `CustomElementRegistry::DefaultRegistry(document)`, perhaps just rename `TreeScope::customElementRegistry` to `TreeScope::DefaultCustomElementRegistry` and change all of the callers to this function to just `document.DefaultCustomElementRegistry()`? (And then delete `CustomElementRegistry::DefaultRegistry`.)
`TreeScope::customElementRegistry()` is used as a mixin in [document_or_shadow_root.idl](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/document_or_shadow_root.idl;l=31), so renaming it would require implementing `customElementRegistry()` on both `Document` and `ShadowRoot`.
Could you please confirm whether this direction is what you intended?
Oh boo, I forgot that was web exposed. Ok, it's fine as-is. Thanks for checking first.
Related details
- Jayson Chen
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen voted Code-Review+1![]( "Open in Gerrit")
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
YeongHan Kim (Gerrit)
YeongHan Kim added 1 comment![]( "Open in Gerrit")
It looks like the Review-Enforcement requirement hasn’t been satisfied yet.
Sorry for the trouble, but could you please take a look and check on this?
Thank you so much. I really appreciate it!
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen added 1 comment![]( "Open in Gerrit")
I think it has something to do with me never setting up reauth properly. Lemme try to fix it.
Related details
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen voted Code-Review+0![]( "Open in Gerrit")
| Code-Review | +0 |
Related details
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- YeongHan Kim
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jayson Chen (Gerrit)
Jayson Chen added 1 comment![]( "Open in Gerrit")
Should be good now. Thanks again for working on this :))
YeongHan Kim (Gerrit)
YeongHan Kim voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Allow the creation of null registry element and shadow root
As discussed in [1], support is being added to allow elements to have
a null customElementRegistry without Shadow DOM. To support this, the
specification has been updated [2] to allow null as a valid argument for
creating elements and shadow roots.
This patch implements that update.
[1] https://github.com/whatwg/dom/issues/1413
[2] https://github.com/whatwg/dom/pull/1424
- M third_party/blink/renderer/core/dom/document.cc
- M third_party/blink/renderer/core/dom/element.cc
- M third_party/blink/renderer/core/dom/element_creation_options.idl
- M third_party/blink/renderer/core/editing/commands/editing_commands_utilities.cc
- M third_party/blink/renderer/core/editing/commands/indent_outdent_command.cc
- M third_party/blink/renderer/core/html/custom/custom_element_registry.cc
- M third_party/blink/renderer/core/html/custom/custom_element_registry.h
- M third_party/blink/renderer/core/html/custom/custom_element_test_helpers.h
- M third_party/blink/renderer/core/html/parser/html_construction_site.cc
- M third_party/blink/renderer/core/xml/parser/xml_document_parser.cc
- M third_party/blink/renderer/core/xml/parser/xml_document_parser_rs.cc
- D third_party/blink/web_tests/external/wpt/custom-elements/Document-createElement-customized-builtins-expected.txt
- D third_party/blink/web_tests/external/wpt/custom-elements/Document-createElementNS-customized-builtins-expected.txt
- D third_party/blink/web_tests/platform/mac/virtual/scoped-custom-element-registry/external/wpt/custom-elements/registries/element-mutation-expected.txt
- M third_party/blink/web_tests/platform/mac/virtual/scoped-custom-element-registry/external/wpt/custom-elements/registries/template.window-expected.txt
- D third_party/blink/web_tests/virtual/scoped-custom-element-registry/external/wpt/custom-elements/Document-createElement-customized-builtins-expected.txt
- D third_party/blink/web_tests/virtual/scoped-custom-element-registry/external/wpt/custom-elements/Document-createElementNS-customized-builtins-expected.txt
- M third_party/blink/web_tests/virtual/scoped-custom-element-registry/external/wpt/custom-elements/registries/Document-createElement-expected.txt
- M third_party/blink/web_tests/virtual/scoped-custom-element-registry/external/wpt/custom-elements/registries/Document-createElementNS-expected.txt
- M third_party/blink/web_tests/virtual/scoped-custom-element-registry/external/wpt/custom-elements/registries/Element-customElementRegistry-expected.txt
- M third_party/blink/web_tests/virtual/scoped-custom-element-registry/external/wpt/custom-elements/registries/ShadowRoot-init-customElementRegistry-expected.txt
- M third_party/blink/web_tests/virtual/scoped-custom-element-registry/external/wpt/custom-elements/registries/scoped-custom-element-registry-customelementregistry-attribute-expected.txt
- M third_party/blink/web_tests/virtual/scoped-custom-element-registry/external/wpt/custom-elements/registries/scoped-registry-append-does-not-upgrade-expected.txt
- M third_party/blink/web_tests/virtual/scoped-custom-element-registry/external/wpt/custom-elements/registries/template.window-expected.txt
Code-Review: +1 by Jayson Chen, +1 by Mason Freed
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |