Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Elements created while parsed inside a fragment think they are connected #11023

Open
AtkinsSJ opened this issue Feb 13, 2025 · 1 comment
Open

Elements created while parsed inside a fragment think they are connected #11023

AtkinsSJ opened this issue Feb 13, 2025 · 1 comment
Labels
topic: forms

Comments

@AtkinsSJ
Copy link
Contributor

What is the issue with the HTML Standard?

While trying to make the last 2 subtests of https://wpt.live/html/semantics/forms/the-input-element/radio-disconnected-group-owner.html pass in Ladybird, we've come across what seems to be a spec bug. I'm not 100% sure of course, but wanted to raise it and see what your opinions are.

Reducing subtest 7 down to this case:

const container = document.createElement("div");
console.log("About to set innerHTML");
container.innerHTML = `<input type="radio" name="group" checked><input type="radio" name="group" checked>`;
console.log("After setting innerHTML");

I get the following in the logs:

5590.176 WebContent(29994): (js log) "About to set innerHTML"
5590.177 WebContent(29994): Inserting input into html
5590.177 WebContent(29994): -> is connected? true
5590.177 WebContent(29994): -> inserted connected radio is checked: true
5590.177 WebContent(29994): Inserting input into html
5590.177 WebContent(29994): -> is connected? true
5590.177 WebContent(29994): -> inserted connected radio is checked: true
5590.177 WebContent(29994): Inserting input into #document-fragment
5590.177 WebContent(29994): -> is connected? false
5590.177 WebContent(29994): Inserting input into #document-fragment
5590.177 WebContent(29994): -> is connected? false
5590.177 WebContent(29994): We parsed a fragment! Is it connected? false
5590.177 WebContent(29994): Inserting input into div
5590.177 WebContent(29994): -> is connected? false
5590.177 WebContent(29994): Inserting input into div
5590.177 WebContent(29994): -> is connected? false
5590.177 WebContent(29994): (js log) "After setting innerHTML"

So what we see is:

  1. First the two inputs are inserted into an HTML element and think they are connected.
  2. Then they're inserted into the document fragment, which correctly is not connected.
  3. Finally they're inserted into the container element, again not connected.

Now, the <input> spec tells us that when the radio buttons are inserted in step 1 above, they need to modify the checkedness of the other radio buttons, because they are connected:

When any of the following phenomena occur, if the element's checkedness state is true after the occurrence, the checkedness state of all the other elements in the same radio button group must be set to false:
...

However, the test expects both radio buttons to remain checked when they are added in this way. So, we should not be connected in step 1.

But as far as I can tell, we're following the spec:

  • The fragment parsing algorithm sets up a document with an <html> element as the root.
  • The HTML parser then creates and inserts the <input> elements.
  • They are connected because "A node becomes connected when the insertion steps are invoked with it as the argument and it is now connected." 1 and "A node is connected if its shadow-including root is a document." 2 This is true because they're being inserted into an <html> element that has a document parent.

It's quite possible there's some nuance here that I'm unaware of, but it seems like there needs to be some exception made that a node is not "connected" if its document is a temporary one used for fragment parsing. I don't know if there would be repercussions to changing that though.

Thanks!
@AtkinsSJ AtkinsSJ mentioned this issue Feb 13, 2025
Merged
@annevk
Copy link
Member

annevk commented Feb 21, 2025

Seems this check was added in #10917. Perhaps it needs to be browsing-context connected?

cc @EdgarChen @domfarolino

# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
topic: forms
Milestone
No milestone
Development

No branches or pull requests
2 participants
@AtkinsSJ @annevk