Last Comment Bug 766569 - CSP lacks l10n support
: CSP lacks l10n support
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: 12 Branch
: x86 Mac OS X
: -- normal (vote)
: mozilla16
Assigned To: Mark Goodwin [:mgoodwin]
:
Mentors:
Depends on: 770176
Blocks: CSP 712859
  Show dependency treegraph
 
Reported: 2012-06-20 08:09 PDT by Mark Goodwin [:mgoodwin]
Modified: 2012-07-10 12:43 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
l10n support for current CSP errors and warnings (31.29 KB, patch)
2012-06-21 10:09 PDT, Mark Goodwin [:mgoodwin]
no flags Details | Diff | Review
l10n support for current CSP errors and warnings (with placeholder l10n text removed) (31.18 KB, patch)
2012-06-21 14:32 PDT, Mark Goodwin [:mgoodwin]
no flags Details | Diff | Review
add l10n support to existing CSP errors and warnings (30.61 KB, patch)
2012-06-22 03:41 PDT, Mark Goodwin [:mgoodwin]
no flags Details | Diff | Review
add l10n support to existing CSP errors and warnings (30.70 KB, patch)
2012-06-28 08:22 PDT, Mark Goodwin [:mgoodwin]
bzbarsky: review+
Details | Diff | Review
add l10n support to existing CSP errors and warnings (fixed) (30.68 KB, patch)
2012-06-29 04:43 PDT, Mark Goodwin [:mgoodwin]
bzbarsky: review+
mozbugs: checkin+
Details | Diff | Review

Description Mark Goodwin [:mgoodwin] 2012-06-20 08:09:49 PDT
Error console messages for CSP Violations cannot currently be localized.
Comment 1 Sid Stamm [:geekboy or :sstamm] 2012-06-20 09:20:35 PDT
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.
Comment 2 Mark Goodwin [:mgoodwin] 2012-06-20 12:07:14 PDT
(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?
Comment 3 Mark Goodwin [:mgoodwin] 2012-06-21 10:09:32 PDT
Created attachment 635353 [details] [diff] [review]
l10n support for current CSP errors and warnings

Is this the right approach? Any comments welcome.
Comment 4 Mark Goodwin [:mgoodwin] 2012-06-21 14:32:48 PDT
Created attachment 635474 [details] [diff] [review]
l10n support for current CSP errors and warnings (with placeholder l10n text removed)

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).
Comment 5 Mark Goodwin [:mgoodwin] 2012-06-22 03:41:14 PDT
Created attachment 635682 [details] [diff] [review]
add l10n support to existing CSP errors and warnings

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?
Comment 6 Mark Goodwin [:mgoodwin] 2012-06-28 08:22:55 PDT
Created attachment 637521 [details] [diff] [review]
add l10n support to existing CSP errors and warnings

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.
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-06-28 09:35:07 PDT
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?
Comment 8 Mark Goodwin [:mgoodwin] 2012-06-28 09:40:17 PDT
(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 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-06-28 09:55:48 PDT
Comment on attachment 637521 [details] [diff] [review]
add l10n support to existing CSP errors and warnings

r=me
Comment 10 Mozilla RelEng Bot 2012-06-29 04:15:27 PDT
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
Comment 11 Mark Goodwin [:mgoodwin] 2012-06-29 04:43:05 PDT
Created attachment 637850 [details] [diff] [review]
add l10n support to existing CSP errors and warnings (fixed)

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.
Comment 12 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-06-29 06:52:27 PDT
Comment on attachment 637850 [details] [diff] [review]
add l10n support to existing CSP errors and warnings (fixed)

r=me
Comment 13 Joe Walker [:jwalker] (needinfo me or ping on irc) 2012-06-29 12:38:41 PDT
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.
Comment 14 Sid Stamm [:geekboy or :sstamm] 2012-06-29 12:52:29 PDT
Pushed to mozilla-inbound.
https://hg.mozilla.org/integration/mozilla-inbound/rev/63212e0a927b
Comment 15 Ryan VanderMeulen [:RyanVM] 2012-06-30 12:43:55 PDT
https://hg.mozilla.org/mozilla-central/rev/63212e0a927b
Comment 16 Axel Hecht [:Pike] 2012-07-05 04:05:12 PDT
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.
Comment 17 Mark Goodwin [:mgoodwin] 2012-07-05 05:43:03 PDT
(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.

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