Closed
Bug 766569
Opened 12 years ago
Closed 12 years ago
CSP lacks l10n support
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: mgoodwin, Assigned: mgoodwin)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 4 obsolete files)
30.68 KB,
patch
|
bzbarsky
:
review+
geekboy
:
checkin+
|
Details | Diff | Splinter Review |
Error console messages for CSP Violations cannot currently be localized.
Comment 1•12 years ago
|
||
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.
Assignee | ||
Comment 2•12 years ago
|
||
(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?
Assignee | ||
Comment 3•12 years ago
|
||
Is this the right approach? Any comments welcome.
Assignee | ||
Comment 4•12 years ago
|
||
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).
Assignee | ||
Updated•12 years ago
|
Attachment #635353 -
Attachment is obsolete: true
Assignee | ||
Comment 5•12 years ago
|
||
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
Assignee | ||
Comment 6•12 years ago
|
||
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 7•12 years ago
|
||
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?
Assignee | ||
Comment 8•12 years ago
|
||
(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 9•12 years ago
|
||
Comment on attachment 637521 [details] [diff] [review] add l10n support to existing CSP errors and warnings r=me
Attachment #637521 -
Flags: review?(bzbarsky) → review+
Comment 10•12 years ago
|
||
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
Assignee | ||
Comment 11•12 years ago
|
||
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 12•12 years ago
|
||
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 13•12 years ago
|
||
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)
Comment 14•12 years ago
|
||
Pushed to mozilla-inbound. https://hg.mozilla.org/integration/mozilla-inbound/rev/63212e0a927b
Target Milestone: --- → mozilla16
Updated•12 years ago
|
Attachment #637850 -
Flags: checkin?(sstamm) → checkin+
Comment 15•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/63212e0a927b
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 16•12 years ago
|
||
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
Updated•12 years ago
|
QA Contact: general
Assignee | ||
Comment 17•12 years ago
|
||
(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.
Description
•