Skip to content

HTML parser: allow <hr> to be used inside <select> as a separator#12407

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
annevk:eng/REGRESSIONHTML5-parser-Menu-separator-hr-doesnt-work-
May 3, 2023
Merged

HTML parser: allow <hr> to be used inside <select> as a separator#12407
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
annevk:eng/REGRESSIONHTML5-parser-Menu-separator-hr-doesnt-work-

Conversation

@annevk

@annevk annevk commented Apr 5, 2023

Copy link
Copy Markdown
Contributor

d9e3a62

HTML parser: allow <hr> to be used inside <select> as a separator
https://bugs.webkit.org/show_bug.cgi?id=80686
rdar://107656886

Reviewed by Ryosuke Niwa.

Revive support in the HTML parser for <hr>-in-<select>, thereby making an existing UI feature much more accessible to web developers.

This reflects a recent change in the HTML Standard: whatwg/html#9124

* LayoutTests/html5lib/resources/webkit02.dat:

These are being upstreamed via html5lib/html5lib-tests#167 and will then be incorporated into web-platform-tests.

* Source/WebCore/html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::processStartTag):

Canonical link: https://commits.webkit.org/263624@main

5c5f004

Misc iOS, tvOS & watchOS macOS Linux Windows
🧪 style 🛠 ios 🛠 mac 🛠 wpe 🛠 wincairo
🧪 bindings 🛠 ios-sim 🛠 mac-AS-debug 🧪 wpe-wk2
🧪 webkitperl 🧪 ios-wk2 🧪 api-mac 🛠 gtk
🧪 ios-wk2-wpt 🧪 mac-wk1 🧪 gtk-wk2
🧪 api-ios 🧪 mac-wk2 🧪 api-gtk
🛠 tv 🧪 mac-AS-debug-wk2
🛠 tv-sim 🧪 mac-wk2-stress
🛠 🧪 merge 🛠 watch
🛠 watch-sim

@annevk annevk self-assigned this Apr 5, 2023
@annevk annevk added the Forms For bugs specific to form elements (checkboxes, buttons, text fields, etc.) label Apr 5, 2023
@annevk annevk changed the title REGRESSION(HTML5 parser?): Menu separator <hr> doesn't work. HTML parser: allow <hr> to be used inside <select> as a separator May 2, 2023
@annevk annevk force-pushed the eng/REGRESSIONHTML5-parser-Menu-separator-hr-doesnt-work- branch from 7fc5303 to 927f7bd Compare May 2, 2023 12:25
@annevk annevk marked this pull request as ready for review May 2, 2023 12:26
@annevk annevk requested review from cdumez and rniwa as code owners May 2, 2023 12:26
@webkit-early-warning-system

webkit-early-warning-system commented May 2, 2023

Copy link
Copy Markdown
Collaborator

@annevk

annevk commented May 2, 2023

Copy link
Copy Markdown
Contributor Author

Note: I plan to make the commit message reflect that the HTML standard has been updated and the tests are being upstreamed in html5lib/html5lib-tests#167, but I'm waiting with that as I don't want to let the bots run just for that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could/should it be an else if ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, that would do the wrong thing. It's equivalent to the case just above this, except that it inserts a void element at the end.

Consider <select><optgroup><option><hr> (one of the tests as well). First <option> is the current item so you pop that off. Then <optgroup> is and you want to pop that off as well. Ultimately inserting <hr> as a child of <select>.

@annevk annevk force-pushed the eng/REGRESSIONHTML5-parser-Menu-separator-hr-doesnt-work- branch from 927f7bd to 5c5f004 Compare May 3, 2023 07:38
@annevk annevk added merge-queue Applied to send a pull request to merge-queue skip-ews Applied to prevent a change from being run on EWS labels May 3, 2023
@webkit-early-warning-system

Copy link
Copy Markdown
Collaborator

Starting EWS tests for 5c5f004. Live statuses available at the PR page, #12407

https://bugs.webkit.org/show_bug.cgi?id=80686
rdar://107656886

Reviewed by Ryosuke Niwa.

Revive support in the HTML parser for <hr>-in-<select>, thereby making an existing UI feature much more accessible to web developers.

This reflects a recent change in the HTML Standard: whatwg/html#9124.

* LayoutTests/html5lib/resources/webkit02.dat:

These are being upstreamed via html5lib/html5lib-tests#167 and will then be incorporated into web-platform-tests.

* Source/WebCore/html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::processStartTag):

Canonical link: https://commits.webkit.org/263624@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSIONHTML5-parser-Menu-separator-hr-doesnt-work- branch from 5c5f004 to d9e3a62 Compare May 3, 2023 08:15
@webkit-commit-queue

Copy link
Copy Markdown
Collaborator

Committed 263624@main (d9e3a62): https://commits.webkit.org/263624@main

Reviewed commits have been landed. Closing PR #12407 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 3, 2023
@webkit-commit-queue webkit-commit-queue merged commit d9e3a62 into WebKit:main May 3, 2023
@annevk annevk deleted the eng/REGRESSIONHTML5-parser-Menu-separator-hr-doesnt-work- branch May 3, 2023 09:36
annevk added a commit to html5lib/html5lib-tests that referenced this pull request May 3, 2023
aarongable pushed a commit to chromium/chromium that referenced this pull request Sep 21, 2023
This is based on this webkit patch:
WebKit/WebKit#12407

https://chromestatus.com/feature/6198448366026752

Bug: 1441883
Change-Id: I47d208edd9367c89bf85fb366c3163094669a286
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4875298
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1199621}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Forms For bugs specific to form elements (checkboxes, buttons, text fields, etc.) skip-ews Applied to prevent a change from being run on EWS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants