[webkit-reviews] review denied: [Bug 229381] Add onsecuritypolicyviolation on SVG and HTML elements : [Attachment 436113] Patch

[bugzilla-daemon at webkit.org]

Frédéric Wang (:fredw) <fred.wang at free.fr> has denied  review:
Bug 229381: Add onsecuritypolicyviolation on SVG and HTML elements
https://bugs.webkit.org/show_bug.cgi?id=229381

Attachment 436113: Patch

https://bugs.webkit.org/attachment.cgi?id=436113&action=review




--- Comment #9 from Frédéric Wang (:fredw) <fred.wang at free.fr> ---
Comment on attachment 436113
  --> https://bugs.webkit.org/attachment.cgi?id=436113
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.