Summary: | Add onsecuritypolicyviolation on GlobalEventHandlers | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sonia Singla <soniasingla.1812> | ||||||||||||||||||||||
Component: | DOM | Assignee: | Sonia Singla <soniasingla.1812> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | cdumez, changseok, clopez, dbarton, esprehn+autocc, ews-watchlist, fred.wang, gyuyoung.kim, kangil.han, kondapallykalyan, mkwst, sam, webkit-bug-importer, youennf | ||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||
Attachments: |
|
Description
Sonia Singla
2021-08-21 22:06:33 PDT
Created attachment 436107 [details]
Patch
:fredw, is it fine to edit LayoutTests/imported/w3c/** to make the tests pass? thanks Created attachment 436112 [details]
Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess Created attachment 436113 [details]
Patch
Comment on attachment 436113 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=436113&action=review > Source/WebCore/dom/ShadowRoot.idl:34 > + attribute EventHandler onsecuritypolicyviolation; I don't see onsecuritypolicyviolation on ShadowRoot in the ShadowRoot spec - https://dom.spec.whatwg.org/#interface-shadowroot. Why did you decide to add this? If this is something other browsers do, we should make sure there is a spec bug about this. (In reply to Sonia Singla from comment #2) > :fredw, is it fine to edit LayoutTests/imported/w3c/** to make the tests > pass? thanks You can edit the results, but not the tests themselves. Instead, the changes to tests should happen in the WPT repository (https://github.com/web-platform-tests/wpt) and then re-imported into LayoutTests. Created attachment 436120 [details]
Patch
Comment on attachment 436113 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=436113&action=review Thanks for the patch. It seems there are more tests passing so you can just update the results, no need to edit the test themselves, right? (if it turns out that the tests are not up-to-date, we would need to sync first). > Source/WebCore/ChangeLog:3 > + Add onsecuritypolicyviolation on ShadowRoot and GlobalEventHandlers Here, in the bug title and elsewhere should just be "GlobalEventHandlers" (not ShadowRoot and not only on SVG/HTML elements). Please also explain the change briefly and add a reference to the spec definition and/or discussion. >> Source/WebCore/dom/ShadowRoot.idl:34 >> + attribute EventHandler onsecuritypolicyviolation; > > I don't see onsecuritypolicyviolation on ShadowRoot in the ShadowRoot spec - https://dom.spec.whatwg.org/#interface-shadowroot. Why did you decide to add this? If this is something other browsers do, we should make sure there is a spec bug about this. yes, this is not in the spec, please remove this. I guess the confusion is because Sonia is working on onslotchange too, which exists on the two interfaces. > LayoutTests/ChangeLog:3 > + Add onsecuritypolicyviolation on ShadowRoot and GlobalEventHandlers Ditto. > LayoutTests/imported/w3c/ChangeLog:3 > + Add onsecuritypolicyviolation on ShadowRoot and GlobalEventHandlers Ditto. Created attachment 436275 [details]
Patch
Created attachment 436277 [details]
Patch
Created attachment 436281 [details]
Patch
Comment on attachment 436281 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=436281&action=review LGTM, with the "PASSS" typo fixed. > LayoutTests/imported/w3c/web-platform-tests/html/dom/idlharness.https-expected.txt:5084 > +PASSS SVGElement interface: attribute onsecuritypolicyviolation "PASS" without extra S. I guess the EWS tests are not failing since we have other idlharness.https-expected.txt for mac, but that's going to fail once we land it. Created attachment 436295 [details]
Patch
Comment on attachment 436295 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=436295&action=review > Source/WebCore/ChangeLog:18 > + Reviewed by NOBODY (OOPS!). Oops, sorry I forgot to say that this line should be after https://bugs.webkit.org/show_bug.cgi?id=229381 (compare with other ChangeLog entries) Created attachment 436379 [details]
Patch
Found 1 new test failure: imported/w3c/web-platform-tests/IndexedDB/serialize-sharedarraybuffer-throws.https.html Created attachment 436391 [details]
Patch
Committed r281569 (240934@main): <https://commits.webkit.org/240934@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 436391 [details]. |