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)
(deleted),
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
|
||
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
•