Skip to content

Conversation

@brentfulgham
Copy link
Contributor

@brentfulgham brentfulgham commented Sep 8, 2022

e30caa2

Handling of text documents does not comply with modern HTML spec
https://bugs.webkit.org/show_bug.cgi?id=244950
<rdar://problem/99719728>

Reviewed by Chris Dumez.

The spec language in https://html.spec.whatwg.org/multipage/browsing-the-web.html#read-text, indicates that Text documents
must be in no-quirks mode: (7.11.4.3 Set document's mode to "no-quirks".)

This is in conflict with the source code in the TextDocument constructor:

  setCompatibilityMode(DocumentCompatibilityMode::QuirksMode);

This causes the following WPT failure:

wpt /html/browsers/browsing-the-web/read-text/load-text-plain.html

* LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/read-text/load-text-plain-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbobjectstore_putall.tentative.any-expected.txt
* LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbobjectstore_putall.tentative.any.worker-expected.txt
* LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/global/value-set.any-expected.txt
* LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/global/value-set.any.worker-expected.txt
* LayoutTests/platform/glib/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/rsa_importKey.https.worker-expected.txt
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/wasm/jsapi/global/value-set.any-expected.txt
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/wasm/jsapi/global/value-set.any.worker-expected.txt
* LayoutTests/platform/gtk/fast/forms/targeted-frame-submission-expected.txt
* LayoutTests/platform/ios-wk2/fast/forms/targeted-frame-submission-expected.txt
* LayoutTests/platform/ios-wk2/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/ios/fast/forms/targeted-frame-submission-expected.txt
* LayoutTests/platform/ios/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/mac/fast/forms/targeted-frame-submission-expected.txt
* LayoutTests/platform/mac/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/mac/http/tests/misc/acid3-expected.txt
* LayoutTests/platform/win/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/wincairo/fast/loader/text-document-wrapping-expected.txt
* Source/WebCore/html/TextDocument.cpp:

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

68cde4c

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

@brentfulgham brentfulgham self-assigned this Sep 8, 2022
@brentfulgham brentfulgham added WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). WebKit Nightly Build labels Sep 8, 2022
@brentfulgham
Copy link
Contributor Author

It will be interesting to see if this change triggers any test failures in our own test cases. Waiting on EWS...

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 9, 2022
@brentfulgham brentfulgham removed the merging-blocked Applied to prevent a change from being merged label Sep 9, 2022
@brentfulgham brentfulgham force-pushed the eng/Handling-of-text-documents-does-not-comply-with-modern-HTML-spec branch from 90a576f to d10634a Compare September 9, 2022 21:14
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 9, 2022
@brentfulgham brentfulgham removed the merging-blocked Applied to prevent a change from being merged label Sep 9, 2022
@brentfulgham brentfulgham force-pushed the eng/Handling-of-text-documents-does-not-comply-with-modern-HTML-spec branch from d10634a to 68cde4c Compare September 9, 2022 22:33
Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

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

LGTM

@brentfulgham brentfulgham added the merge-queue Applied to send a pull request to merge-queue label Sep 12, 2022
https://bugs.webkit.org/show_bug.cgi?id=244950
<rdar://problem/99719728>

Reviewed by Chris Dumez.

The spec language in https://html.spec.whatwg.org/multipage/browsing-the-web.html#read-text, indicates that Text documents
must be in no-quirks mode: (7.11.4.3 Set document's mode to "no-quirks".)

This is in conflict with the source code in the TextDocument constructor:

  setCompatibilityMode(DocumentCompatibilityMode::QuirksMode);

This causes the following WPT failure:

wpt /html/browsers/browsing-the-web/read-text/load-text-plain.html

* LayoutTests/imported/w3c/web-platform-tests/html/browsers/browsing-the-web/read-text/load-text-plain-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbobjectstore_putall.tentative.any-expected.txt
* LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idbobjectstore_putall.tentative.any.worker-expected.txt
* LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/global/value-set.any-expected.txt
* LayoutTests/imported/w3c/web-platform-tests/wasm/jsapi/global/value-set.any.worker-expected.txt
* LayoutTests/platform/glib/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/rsa_importKey.https.worker-expected.txt
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/wasm/jsapi/global/value-set.any-expected.txt
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/wasm/jsapi/global/value-set.any.worker-expected.txt
* LayoutTests/platform/gtk/fast/forms/targeted-frame-submission-expected.txt
* LayoutTests/platform/ios-wk2/fast/forms/targeted-frame-submission-expected.txt
* LayoutTests/platform/ios-wk2/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/ios/fast/forms/targeted-frame-submission-expected.txt
* LayoutTests/platform/ios/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/mac/fast/forms/targeted-frame-submission-expected.txt
* LayoutTests/platform/mac/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/mac/http/tests/misc/acid3-expected.txt
* LayoutTests/platform/win/fast/loader/text-document-wrapping-expected.txt
* LayoutTests/platform/wincairo/fast/loader/text-document-wrapping-expected.txt
* Source/WebCore/html/TextDocument.cpp:

Canonical link: https://commits.webkit.org/254389@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Handling-of-text-documents-does-not-comply-with-modern-HTML-spec branch from 68cde4c to e30caa2 Compare September 12, 2022 16:55
@webkit-commit-queue
Copy link
Collaborator

Committed 254389@main (e30caa2): https://commits.webkit.org/254389@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit e30caa2 into WebKit:main Sep 12, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants