Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

appendChild isn't upgrading a custom element #25009

Closed
pshaughn opened this issue Dec 2, 2019 · 5 comments
Closed

appendChild isn't upgrading a custom element #25009

pshaughn opened this issue Dec 2, 2019 · 5 comments

Comments

@pshaughn
Copy link
Member

@pshaughn pshaughn commented Dec 2, 2019

WPT custom-elements/Document-createElement.html creates an element with a 'my-div' is, then registers 'my-div', then appends the element, and the element has not become an instance of the registered class.

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 2, 2019

There are a lot of test cases; this one is the assert_true with 'Undefined element is upgraded on connecting to a document'

@jdm jdm added the A-content/dom label Dec 2, 2019
@jdm jdm added this to To do in web-platform-test failures via automation Dec 2, 2019
@pshaughn pshaughn self-assigned this Dec 23, 2019
@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 23, 2019

get_is is returning None on the element in question, so try_upgrade_element doesn't proceed to enqueue an upgrade. Possibly this has something in common with #25008 regarding the handling of ElementCreationOptions.

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 23, 2019

Adding to the end of Document::CreateElement like so:

	warn!("Creating {:?} element with is {:?}",name,is);
        let el=Element::create(
            name,
            is,
            self,
            ElementCreator::ScriptCreated,
            CustomElementCreationMode::Synchronous,
        );
	warn!("Created element with is {:?}",el.get_is());
	Ok(el)

I observe:

 0:11.68 pid:18583 [2019-12-23T00:46:20Z WARN  script::dom::document] Creating QualName { prefix: None, ns: Atom('http://www.w3.org/1999/xhtml' type=static), local: Atom('div' type=static) } element with is Some(Atom('my-div' type=inline))
 0:11.68 pid:18583 [2019-12-23T00:46:20Z WARN  script::dom::document] Created element with is None
@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 23, 2019

I think I have it pinned down now, https://dom.spec.whatwg.org/#concept-create-element uses the is value as a string in 7.2 even when there's not currently a definition on the name, but

// Steps 7.1-7.2
let result = create_native_html_element(name.clone(), prefix, document, creator);
does not. It looks like just adding a set_is right after create_native_html_element will clear everything up.

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Dec 23, 2019

It also clears up #24997 and #24998! It is not solving #24993 AFAICT.

bors-servo added a commit that referenced this issue Dec 23, 2019
apply is: to Document.createElement even when name isn't registered yet

The "is" option to Document.createElement should be respected even when the name hasn't been registered yet, in which case the name gets looked up again at the time the element should be upgraded. This change does that.
I'm now seeing a few test timeouts that aren't in the metadata, but I suspect they're slowness on my local configuration and not actual breakage.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25009 fix #24997 and fix #24998

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Dec 23, 2019
apply is: to Document.createElement even when name isn't registered yet

The "is" option to Document.createElement should be respected even when the name hasn't been registered yet, in which case the name gets looked up again at the time the element should be upgraded. This change does that.
I'm now seeing a few test timeouts that aren't in the metadata, but I suspect they're slowness on my local configuration and not actual breakage.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25009 fix #24997 and fix #24998

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Dec 23, 2019
apply is: to Document.createElement even when name isn't registered yet

The "is" option to Document.createElement should be respected even when the name hasn't been registered yet, in which case the name gets looked up again at the time the element should be upgraded. This change does that.
I'm now seeing a few test timeouts that aren't in the metadata, but I suspect they're slowness on my local configuration and not actual breakage.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25009 fix #24997 and fix #24998

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Dec 23, 2019
apply is: to Document.createElement even when name isn't registered yet

The "is" option to Document.createElement should be respected even when the name hasn't been registered yet, in which case the name gets looked up again at the time the element should be upgraded. This change does that.
I'm now seeing a few test timeouts that aren't in the metadata, but I suspect they're slowness on my local configuration and not actual breakage.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25009 fix #24997 and fix #24998

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Dec 23, 2019
apply is: to Document.createElement even when name isn't registered yet

The "is" option to Document.createElement should be respected even when the name hasn't been registered yet, in which case the name gets looked up again at the time the element should be upgraded. This change does that.
I'm now seeing a few test timeouts that aren't in the metadata, but I suspect they're slowness on my local configuration and not actual breakage.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25009 fix #24997 and fix #24998

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Dec 23, 2019
apply is: to Document.createElement even when name isn't registered yet

The "is" option to Document.createElement should be respected even when the name hasn't been registered yet, in which case the name gets looked up again at the time the element should be upgraded. This change does that.
I'm now seeing a few test timeouts that aren't in the metadata, but I suspect they're slowness on my local configuration and not actual breakage.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25009 fix #24997 and fix #24998

<!-- Either: -->
- [X] There are tests for these changes OR

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
web-platform-test failures automation moved this from To do to Done Dec 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.