Skip to content

Conversation

@codesculpture
Copy link
Member

@codesculpture codesculpture commented Aug 23, 2024

0afcbca

Serializing children of void elements
https://bugs.webkit.org/show_bug.cgi?id=277440

Reviewed by Anne van Kesteren.

As per XML, if void elements contains children, the children also gets serialized,
as void elements concepts are only valid in HTML, So checking whether the syntax is xml, then it must have close tag except if it doesn't have children

* LayoutTests/imported/w3c/web-platform-tests/domparsing/XMLSerializer-serializeToString-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/domparsing/XMLSerializer-serializeToString.html:
* Source/WebCore/editing/MarkupAccumulator.cpp:
(WebCore::MarkupAccumulator::serializeNodesWithNamespaces):

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

0a23097

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

@codesculpture codesculpture requested a review from rniwa as a code owner August 23, 2024 16:41
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 23, 2024
@codesculpture
Copy link
Member Author

@annevk seems some of the test were failing already (despite this changes) in LayoutTests/imported/w3c/web-platform-tests/domparsing/XMLSerializer-serializeToString.html

But am sure the test case i added would pass across all, but i cannot take specific logs regarding that.

@Ahmad-S792
Copy link
Contributor

@annevk seems some of the test were failing already (despite this changes) in LayoutTests/imported/w3c/web-platform-tests/domparsing/XMLSerializer-serializeToString.html

But am sure the test case i added would pass across all, but i cannot take specific logs regarding that.

You can use run-webkit-tests <path> --reset-results --no-build to generate test expectation for the test case and you should squash both changes into one commit.

Once you updated WPT test case, you can export them through script present in Tools/Scripts but @annevk would know how exactly to do (never did myself).

@codesculpture
Copy link
Member Author

@Ahmad-S792 thanks will try that.

@codesculpture codesculpture force-pushed the fix/serialize_xml/void branch from fc00fd8 to ec32397 Compare August 24, 2024 16:11
@codesculpture codesculpture force-pushed the fix/serialize_xml/void branch from ec32397 to 0a23097 Compare August 25, 2024 12:38
@codesculpture
Copy link
Member Author

Unable to determine email address for ['codesculpture'] from metadata/contributors.json. Skipping sending email.

I see only this in stderr to check why test is failed.
But seems the title of the error is something in w3c test case was not passed. But log is irrelevant to it. Any idea @annevk

@annevk annevk removed the merging-blocked Applied to prevent a change from being merged label Aug 26, 2024
@annevk annevk added safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks merge-queue Applied to send a pull request to merge-queue and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Aug 26, 2024
https://bugs.webkit.org/show_bug.cgi?id=277440

Reviewed by Anne van Kesteren.

As per XML, if void elements contains children, the children also gets serialized,
as void elements concepts are only valid in HTML, So checking whether the syntax is xml, then it must have close tag except if it doesn't have children

* LayoutTests/imported/w3c/web-platform-tests/domparsing/XMLSerializer-serializeToString-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/domparsing/XMLSerializer-serializeToString.html:
* Source/WebCore/editing/MarkupAccumulator.cpp:
(WebCore::MarkupAccumulator::serializeNodesWithNamespaces):

Canonical link: https://commits.webkit.org/282725@main
@webkit-commit-queue
Copy link
Collaborator

Committed 282725@main (0afcbca): https://commits.webkit.org/282725@main

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

@webkit-commit-queue webkit-commit-queue merged commit 0afcbca into WebKit:main Aug 26, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants