Skip to content
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

Jump to bottom

Implement trusted types integrations with DOM attribute APIs. #26519

Merged
merged 1 commit into from
May 15, 2024
Merged

Implement trusted types integrations with DOM attribute APIs. #26519

merged 1 commit into from
May 15, 2024

Conversation

ziransun
Copy link
Contributor

@ziransun ziransun commented Mar 27, 2024 •
edited by webkit-early-warning-system
Loading

9a38b69

Implement trusted types integrations with DOM attribute APIs.
https://bugs.webkit.org/show_bug.cgi?id=270436

Reviewed by Ryosuke Niwa.

Implement the spec updates at whatwg/dom#1247

It also removes some expectations in GTK as the results should be
in line with the general expectation file.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/Element-setAttribute-respects-Elements-node-documents-globals-CSP-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/GlobalEventHandlers-onclick-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/TrustedTypePolicyFactory-metadata.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Element-setAttribute-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Element-setAttributeNS-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/trusted-types-event-handlers-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/trusted-types-event-handlers.html:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/trusted-types-svg-script-set-href-expected.txt:
* LayoutTests/platform/gtk/TestExpectations:
* Source/WebCore/dom/Element.cpp:
(WebCore::trustedTypesCompliantAttributeValue):
(WebCore::Element::validateAttributeIndex const):
(WebCore::Element::toggleAttribute):
(WebCore::Element::setAttribute):
(WebCore::Element::setElementsArrayAttribute):
(WebCore::appendAttributes):
(WebCore::Element::setAttributeNode):
(WebCore::Element::setAttributeNodeNS):
(WebCore::Element::setAttributeNS):
* Source/WebCore/dom/Element.h:
* Source/WebCore/dom/Element.idl:
* Source/WebCore/dom/TrustedScript.h:
* Source/WebCore/dom/TrustedScriptURL.h:
(WebCore::TrustedScriptURL::toString const): Deleted.
(WebCore::TrustedScriptURL::toJSON const): Deleted.
* Source/WebCore/dom/TrustedType.cpp:
(WebCore::stringToTrustedType):
(WebCore::trustedTypeForAttribute):
* Source/WebCore/dom/TrustedType.h:
* Source/WebCore/dom/TrustedTypePolicyFactory.cpp:
(WebCore::TrustedTypePolicyFactory::getAttributeType const):
* Source/WebKit/WebProcess/InjectedBundle/API/mac/WKDOMElement.mm:
(-[WKDOMElement setAttribute:value:]):
* Source/WebKitLegacy/mac/DOM/DOMElement.mm:
(-[DOMElement setAttribute:value:]):
(-[DOMElement setAttributeNS:qualifiedName:value:]):

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

04b9b37

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 ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ⏳ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 27, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 44b38f4)

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

@lukewarlow lukewarlow marked this pull request as draft March 27, 2024 16:24
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 27, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 28, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 0d8e713)

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 ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-skia
❌ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 28, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 50bafe9)

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 ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-skia
❌ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Mar 29, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 7c67e83)

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 ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-skia
❌ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
❌ 🛠 watch
❌ 🛠 watch-sim

@lukewarlow
Copy link
Member

I think we need to add WEBCORE_EXPORT to TrustedScript and TrustedScriptURL like we have for TrustedHTML, that should fix the build failure.

lukewarlow
lukewarlow requested changes Apr 11, 2024
View reviewed changes
Source/WebCore/dom/Element.cpp
if (attributeValue.hasException())
return attributeValue.releaseException();

setAttributeInternal(index, name, AtomString(attributeValue.releaseReturnValue()), InSynchronizationOfLazyAttribute::No);
Copy link
Member

Choose a reason for hiding this comment

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

This index can be stale as getTrustedTypesCompliantAttributeValue can result in JS execution which may mutate the attributes of the element. We need to work out an interopable solution to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed, we will look this at a later stage. The bug is filed at https://bugs.webkit.org/show_bug.cgi?id=272678

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 13, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 50c70b8)

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 ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ⏳ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

@ziransun ziransun mentioned this pull request May 13, 2024
Merged
@lukewarlow lukewarlow self-requested a review May 13, 2024 18:01
lukewarlow
lukewarlow previously requested changes May 13, 2024
View reviewed changes
Copy link
Member

@lukewarlow lukewarlow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Would be good to try and get rid of the header imports in the Element.h file.

Edit: Done

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 14, 2024 •
edited
Loading

EWS run on previous version of this PR (hash ce61832)

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 ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ⏳ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 14, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 405cb60)

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 ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ⏳ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 watch
✅ 🛠 watch-sim

@lukewarlow lukewarlow marked this pull request as ready for review May 14, 2024 13:32
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 14, 2024
@lukewarlow lukewarlow removed the merging-blocked Applied to prevent a change from being merged label May 15, 2024
@lukewarlow lukewarlow requested a review from rniwa May 15, 2024 10:42
@lukewarlow lukewarlow marked this pull request as draft May 15, 2024 11:01
@lukewarlow lukewarlow changed the title Implement trusted types integrations with DOM setAttribute APIs. Implement trusted types integrations with DOM attribute APIs. May 15, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 15, 2024 •
edited
Loading

EWS run on current version of this PR (hash 04b9b37)

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 ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ⏳ 🛠 wpe-skia
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@lukewarlow lukewarlow marked this pull request as ready for review May 15, 2024 14:58
@lukewarlow lukewarlow 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 May 15, 2024
@ziransun @lukewarlow
Implement trusted types integrations with DOM attribute APIs.
9a38b69 
https://bugs.webkit.org/show_bug.cgi?id=270436

Reviewed by Ryosuke Niwa.

Implement the spec updates at whatwg/dom#1247

It also removes some expectations in GTK as the results should be
in line with the general expectation file.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/Element-setAttribute-respects-Elements-node-documents-globals-CSP-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/GlobalEventHandlers-onclick-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/TrustedTypePolicyFactory-metadata.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Element-setAttribute-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/block-string-assignment-to-Element-setAttributeNS-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/trusted-types-event-handlers-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/trusted-types-event-handlers.html:
* LayoutTests/imported/w3c/web-platform-tests/trusted-types/trusted-types-svg-script-set-href-expected.txt:
* LayoutTests/platform/gtk/TestExpectations:
* Source/WebCore/dom/Element.cpp:
(WebCore::trustedTypesCompliantAttributeValue):
(WebCore::Element::validateAttributeIndex const):
(WebCore::Element::toggleAttribute):
(WebCore::Element::setAttribute):
(WebCore::Element::setElementsArrayAttribute):
(WebCore::appendAttributes):
(WebCore::Element::setAttributeNode):
(WebCore::Element::setAttributeNodeNS):
(WebCore::Element::setAttributeNS):
* Source/WebCore/dom/Element.h:
* Source/WebCore/dom/Element.idl:
* Source/WebCore/dom/TrustedScript.h:
* Source/WebCore/dom/TrustedScriptURL.h:
(WebCore::TrustedScriptURL::toString const): Deleted.
(WebCore::TrustedScriptURL::toJSON const): Deleted.
* Source/WebCore/dom/TrustedType.cpp:
(WebCore::stringToTrustedType):
(WebCore::trustedTypeForAttribute):
* Source/WebCore/dom/TrustedType.h:
* Source/WebCore/dom/TrustedTypePolicyFactory.cpp:
(WebCore::TrustedTypePolicyFactory::getAttributeType const):
* Source/WebKit/WebProcess/InjectedBundle/API/mac/WKDOMElement.mm:
(-[WKDOMElement setAttribute:value:]):
* Source/WebKitLegacy/mac/DOM/DOMElement.mm:
(-[DOMElement setAttribute:value:]):
(-[DOMElement setAttributeNS:qualifiedName:value:]):

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

Committed 278817@main (9a38b69): https://commits.webkit.org/278817@main

Reviewed commits have been landed. Closing PR #26519 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 15, 2024
@webkit-commit-queue webkit-commit-queue merged commit 9a38b69 into WebKit:main May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rniwa rniwa rniwa approved these changes

@lukewarlow lukewarlow lukewarlow left review comments

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

Assignees

@ziransun ziransun

Labels
DOM For bugs specific to XML/HTML DOM elements (including parsing).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ziransun @webkit-early-warning-system @lukewarlow @rniwa @webkit-commit-queue @webkit-ews-buildbot