Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: mgoodwin, Assigned: mgoodwin)

Tracking

(Blocks 1 bug)

12 Branch
mozilla16
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Error console messages for CSP Violations cannot currently be localized.
We should wait until CSP 1.0 is standardized by W3C before we freeze any strings for localization -- that way we won't have to re-translate anything if the spec causes the strings to change.
(In reply to Sid Stamm [:geekboy] from comment #1)
> We should wait until CSP 1.0 is standardized by W3C before we freeze any
> strings for localization -- that way we won't have to re-translate anything
> if the spec causes the strings to change.

This doesn't stop us working on localized string support though, does it?
Is this the right approach? Any comments welcome.
Fixed a potential issue with formatStringFromName (according to https://developer.mozilla.org/en/nsIStringBundle, the number of strings must be the same in the call and the properties file). Also removed placeholder text from the properties file (now strings for en-US are identical, down to spelling errors, to the previous hardcoded entries).
Attachment #635353 - Attachment is obsolete: true
CSPLocalizer borrows from WebConsoleUtils.l10n in browser/devtools/webconsole/WebConsoleUtils.jsm (it's a useful way of seeing a failure is due to an issue with getting the localized string). Is it ok to do this?
Attachment #635474 - Attachment is obsolete: true
This leaves CSP functionality completely unchanged; it simply takes the strings from CSPUtils.jsm and contentSecurityPolicy.js and moves them to a properties file.

Latest version updated to work with the webconsole CSP changes.
Attachment #635682 - Attachment is obsolete: true
Attachment #637521 - Flags: review?(bzbarsky)
Comment on attachment 637521 [details] [diff] [review]
add l10n support to existing CSP errors and warnings

Is this changing just strings that appear in our error console, or also reports sent to servers?
(In reply to Boris Zbarsky (:bz) from comment #7)
> Comment on attachment 637521 [details] [diff] [review]
> add l10n support to existing CSP errors and warnings
> 
> Is this changing just strings that appear in our error console, or also
> reports sent to servers?

These are just the strings in calls to CSPWarning and CSPError, so this is only for error console (and web console).
Comment on attachment 637521 [details] [diff] [review]
add l10n support to existing CSP errors and warnings

r=me
Attachment #637521 - Flags: review?(bzbarsky) → review+
Try run for 22a2ff7b02c3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=22a2ff7b02c3
Results (out of 227 total builds):
    exception: 1
    success: 193
    warnings: 31
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mgoodwin@mozilla.com-22a2ff7b02c3
I noticed a problem with contentSecurityPolicy.js:558 - the reference to this.innerWindowID was a) in the wrong place (should have been a param to CSPWarning, not getFormatStr) and b) wouldn't work anyway ('this' is the CSPReportRedirectSink not ContentSecurityPolicy).

I'll be working at getting non-violation related CSP errors and warnings into the WebConsole as my next piece of work on bug 712859 so I've just removed it for for the time being.
Attachment #637521 - Attachment is obsolete: true
Attachment #637850 - Flags: review?(bzbarsky)
Comment on attachment 637850 [details] [diff] [review]
add l10n support to existing CSP errors and warnings (fixed)

r=me
Attachment #637850 - Flags: review?(bzbarsky) → review+
Comment on attachment 637850 [details] [diff] [review]
add l10n support to existing CSP errors and warnings (fixed)

On behalf of Mark, https://tbpl.mozilla.org/?tree=Try&rev=4f2a0156a99b looks fairly green to me.
Attachment #637850 - Flags: checkin?(sstamm)
Pushed to mozilla-inbound.
https://hg.mozilla.org/integration/mozilla-inbound/rev/63212e0a927b
Target Milestone: --- → mozilla16
Attachment #637850 - Flags: checkin?(sstamm) → checkin+
https://hg.mozilla.org/mozilla-central/rev/63212e0a927b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Depends on: 770176
FWIW, we shouldn't extract strings without reviewing them for proper English. Having follow up bugs like 771076 makes life harder than necessary.

I personally poke Matej Novak in case of doubt on English strings and use of language, did so now in the follow up bug.
QA Contact: general
QA Contact: general
(In reply to Axel Hecht [:Pike] from comment #16)
> FWIW, we shouldn't extract strings without reviewing them for proper
> English. Having follow up bugs like 771076 makes life harder than necessary.

Duly noted.
You need to log in before you can comment on or make changes to this bug.