[webkit-reviews] review granted: [Bug 86323] Content Security Policy console error messages should include the violated directive. : [Attachment 141612] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Mike West <mkwst at chromium.org>'s
request for review:
Bug 86323: Content Security Policy console error messages should include the
violated directive.
https://bugs.webkit.org/show_bug.cgi?id=86323

Attachment 141612: Patch
https://bugs.webkit.org/attachment.cgi?id=141612&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=141612&action=review


> Source/WebCore/page/ContentSecurityPolicy.cpp:661
> +    const String verb = type == "connect" ? ("connect to") : ("load the");

Should just be String rather than const String. Also no need for those
parentheses; it’s not a conventional style for something like this. Maybe
there’s another way to do this without constructing this extra string that we
have to destroy, but I can’t think of anything offhand.

> Source/WebCore/page/ContentSecurityPolicy.cpp:662
> +    reportViolation(directive->text(), "Refused to " + verb + " " + type + "
'" + url.string() + "' as it violates the following Content Security Policy
directive: \"" + directive->text() + "\".\n", url);

I’m not too happy with the phrase “refused to x as it violates y”; the word
“as” there does not seem right to me. I think the word “because” is far
clearer. Or maybe “since”.