[webkit-reviews] review granted: [Bug 237440] CSP report does not get sent to the document in the case of a detached element : [Attachment 453785] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Kate Cheney
<katherine_cheney at apple.com>'s request for review:
Bug 237440: CSP report does not get sent to the document in the case of a
detached element
https://bugs.webkit.org/show_bug.cgi?id=237440

Attachment 453785: Patch

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




--- Comment #6 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 453785
  --> https://bugs.webkit.org/attachment.cgi?id=453785
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=453785&action=review

r=me with suggestions.

> Source/WebCore/dom/Element.cpp:3257
> +	      
document().enqueueSecurityPolicyViolationEvent(WTFMove(eventInit));

can we just do this here?
document().dispatchEvent(SecurityPolicyViolationEvent::create(eventNames().secu
ritypolicyviolationEvent, WTFMove(eventInit), Event::IsTrusted::Yes));

We've already queued a task to get here. Seems odd to re-queue again inside
enqueueSecurityPolicyViolationEvent().

> Source/WebCore/dom/Element.cpp:3259
> +	       auto event =
SecurityPolicyViolationEvent::create(eventNames().securitypolicyviolationEvent,
WTFMove(eventInit), Event::IsTrusted::Yes);

nit: I don't think this local variable is very useful. I personally feel it
would look nicer if we just did this on one line without curly brackets.