The CSP messages in the devtools say that a resource was blocked even when it was not (e.g., in report only mode). This is likely due to my change for CSP messages in bug 607067 :/ (Screenshot: http://i.imgur.com/3R8CPZ1.png)

> This is likely due to my change for CSP messages in bug 607067 :/ I don't think so, it looks like we never differentiated the console messages for violations for CSP vs. CSP-RO. This should be an easy fix: 1. add some strings to csp.properties with language for report-only headers 2. distinguish CSP from CSP-RO in CSPUtils.jsm::logToConsole by checking the _reportOnlyMode flag on the CSPRep Do you think have time to tackle this freddyb? Otherwise I can take it, but won't get to it until next week. Additionally, we'll need to fix this in the new implementation, which is landing very soon.

Ah, I couldn't find logToConsole in CSPUtils.jsm. It is in contentsecuritypolicy.js! :) I will report back with a first attempt quite soon.

This should solve it. I have tested it locally and it seems to work quite well. try push: https://tbpl.mozilla.org/?tree=Try&rev=b1406a452ab4 I'm not super happy with the string and open to suggestions.

CCing dolske and flod since they provided some valuable feedback on the strings in bug 607067, which was quite similar.

Same patch, using `let` instead of `var`.

try looks good. no need to re-test with let instead of var ;)

Comment on attachment 8424831 [details] [diff] [review] bug-1010953.patch Review of attachment 8424831 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed. ::: content/base/src/contentSecurityPolicy.js @@ +644,2 @@ > } else { > + let localizeString = policy._reportOnlyMode ? "CSPROViolation" : "CSPViolation" Nit: both lines should have a semicolon at the end ::: dom/locales/en-US/chrome/security/csp.properties @@ +11,5 @@ > # %2$S is the URI of the resource which violated the directive. > CSPViolationWithURI = The page's settings blocked the loading of a resource at %2$S ("%1$S"). > +# LOCALIZATION NOTE (CSPROViolation): > +# %1$S is the reason why the resource has not been loaded. > +CSPROViolation = The page's settings observed the loading of a resource ("%1$S"). A CSP report is being sent. Suggested language: "A violation occurred for a report-only CSP policy. The behavior was allowed, and a CSP report was sent." Note that "loading of a resource" might be confusing for CSPROViolation because it can refer to other violations that do not have to do with resource loads, such as inline script/style blocking. It makes more sense for CSPROViolationWithURI.

Addressing the string suggestion while carrying over r+ from grobinson.

Please check in. A previous version (different localization string) has successfully run on try yesterday, see here: https://tbpl.mozilla.org/?tree=Try&rev=b1406a452ab4

Freddy, thanks for filing the bug and providing the patch. I just chatted with Garret and we think it would be great to have a testcase for that change though. I think we should have a file loading with 2 policies (one CSP and and CSP-report-only), you can simply load 2 scripts, one that is allowed by CSP and one only by CSP-RO; afterwards just check that we correctly log to the console. Freddy, any chance you want to write the test as well?

It seems the test would be very similar to http://dxr.mozilla.org/mozilla-central/source/browser/devtools/webconsole/test/browser_webconsole_bug_770099_violation.js wouldn't it? I will provide the test if you file the bug and assign it to me, deal? ;)

(In reply to Frederik Braun [:freddyb] from comment #13) > It seems the test would be very similar to > http://dxr.mozilla.org/mozilla-central/source/browser/devtools/webconsole/ > test/browser_webconsole_bug_770099_violation.js wouldn't it? > I will provide the test if you file the bug and assign it to me, deal? ;) Indeed that would be very similar to that testcase. And, it's a deal, here is the bugnumber: 1013507 Thanks!