Bug 229381 – Add onsecuritypolicyviolation on GlobalEventHandlers

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Sonia Singla]

See https://github.com/whatwg/html/pull/2651

[Sonia Singla]

Created attachment 436107 [details]
Patch

[Sonia Singla]

:fredw, is it fine to edit LayoutTests/imported/w3c/**  to make the tests pass? thanks

[Sonia Singla]

Created attachment 436112 [details]
Patch

[EWS Watchlist]

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

[Sonia Singla]

Created attachment 436113 [details]
Patch

[Sam Weinig]

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.

[Sam Weinig]

(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.

[Sonia Singla]

Created attachment 436120 [details]
Patch

[Frédéric Wang (:fredw)]

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.

[Sonia Singla]

Created attachment 436275 [details]
Patch

[Sonia Singla]

Created attachment 436277 [details]
Patch

[Sonia Singla]

Created attachment 436281 [details]
Patch

[Frédéric Wang (:fredw)]

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.

[Sonia Singla]

Created attachment 436295 [details]
Patch

[Frédéric Wang (:fredw)]

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)

[Sonia Singla]

Created attachment 436379 [details]
Patch

[EWS]

Found 1 new test failure: imported/w3c/web-platform-tests/IndexedDB/serialize-sharedarraybuffer-throws.https.html

[Sonia Singla]

Created attachment 436391 [details]
Patch

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/82350346>