Skip to content

selectedcontent element mutation record test is wrong #55849

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.

Sign up for GitHub

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

Open
annevk wants to merge 1 commit into
base: master
Choose a base branch
from
Open

selectedcontent element mutation record test is wrong #55849

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions html/semantics/forms/the-select-element/customizable-select/selectedcontent-mutations.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,14 +301,14 @@
"Type: childList | Target: selectedcontent | Added: [#text: \"\"] | After: selectedcontent",
"Type: childList | Target: button | Added: [#text: \"\"] | After: selectedcontent",
"Type: childList | Target: button | Added: [selectedcontent] | After: #text: \"\"",
"Type: childList | Target: selectedcontent | Removed: [#text: \"outer1\", selectedcontent, #text: \"\"]",
"Type: childList | Target: selectedcontent | Added: [#text: \"outer2\"]",
"Type: childList | Target: selectedcontent | Added: [selectedcontent] | After: #text: \"outer2\"",
"Type: childList | Target: selectedcontent | Added: [#text: \"inner2\"]",
"Type: childList | Target: selectedcontent | Added: [#text: \"\"] | After: selectedcontent",
"Type: childList | Target: button | Added: [#text: \"\"] | After: selectedcontent",
"Type: childList | Target: select | Added: [#text: \"\"] | After: button",
"Type: childList | Target: select | Added: [option] | After: #text: \"\"",
"Type: childList | Target: selectedcontent | Removed: [#text: \"outer1\", selectedcontent, #text: \"\"]",
Copy link
Copy Markdown
Contributor

@josepharhar josepharhar Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this "outer1" up matches my expectations: https://chromium-review.googlesource.com/c/chromium/src/+/7127725/11/third_party/blink/web_tests/external/wpt/html/semantics/forms/the-select-element/customizable-select/selectedcontent-mutations.html

"Type: childList | Target: option | Added: [#text: \"option\"]",
"Type: childList | Target: selectedcontent | Added: [#text: \"option\"]",
"Type: childList | Target: select | Added: [#text: \"\"] | After: option",
Expand Down Expand Up @@ -347,9 +347,8 @@
"Type: childList | Target: select | Added: [div] | After: #text: \"\"",
"Type: childList | Target: div | Added: [#text: \"\"]",
"Type: childList | Target: div | Added: [option] | After: #text: \"\"",
"Type: childList | Target: selectedcontent | Removed: [#text: \"one\"]",
"Type: childList | Target: option | Added: [#text: \"two\"]",
"Type: childList | Target: selectedcontent | Added: [#text: \"two\"]",
"Type: childList | Target: selectedcontent | Added: [#text: \"two\"] | Removed: [#text: \"one\"]",
Copy link
Copy Markdown
Contributor

@josepharhar josepharhar Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this change should be necessary after whatwg/html#11890 but if you want to wait for that PR to be merged first then that's ok

Copy link
Copy Markdown
Member Author

@annevk annevk Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure that is the approach we want. Less mutations seems better. WebKit has a single code path for selecting an option HTMLSelectElement::selectOption() which is where I update the selectedcontent element now. That seems to be doing the right thing in all cases and result in fewer mutations here.

Copy link
Copy Markdown
Contributor

@josepharhar josepharhar Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the interest of keeping things moving, I think we can merge this.

I think this test will be significantly rebaselined after we make every selectedcontent element stay updated, so I anticipate we may revisit this later.

"Type: childList | Target: div | Added: [#text: \"\"] | After: option",
"Type: childList | Target: select | Added: [#text: \"\"] | After: div",
"Type: childList | Target: div#test4 | Added: [#text: \"\"] | After: select"
Expand Down