Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(13)

Issue 2681623002: [i18n] $i18nPolymer to backslash escape (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 2 weeks ago by dschuyler
Modified:
5 months, 1 week ago
CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org, arv+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[i18n] $i18nPolymer to backslash escape This CL improves security and properly displays i18n strings containing quotes. This will backslash escape single quotes, double quotes, and backslashes. BUG=688254 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2681623002 Cr-Commit-Position: refs/heads/master@{#450407} Committed: https://chromium.googlesource.com/chromium/src/+/0e79f6961d70e8f90eb87ecf355efbb7a55b777c

Patch Set 1 #

Patch Set 2 : unit tests #

Patch Set 3 : comment #

Patch Set 4 : clean up #

Patch Set 5 : i18nStr to i18nPolymer #

Patch Set 6 : merge with master #

Total comments: 4

Patch Set 7 : nit #

Total comments: 2

Patch Set 8 : moo #

Messages

Total messages: 52 (35 generated)
dschuyler
5 months, 2 weeks ago (2017-02-07 02:35:04 UTC) #8
Dan Beam (no longer on Chrome)
can we just not do this '$i18n*{...}' thing instead?
5 months, 2 weeks ago (2017-02-08 05:33:18 UTC) #12
dschuyler
> can we just not do this '$i18n*{...}' thing instead? If we change to something ...
5 months, 2 weeks ago (2017-02-08 19:25:23 UTC) #15
dschuyler
We chatted in person about changing this to i18nPolymer, this recent patch makes that change.
5 months, 2 weeks ago (2017-02-11 00:51:12 UTC) #23
Dan Beam (no longer on Chrome)
how do we end up in situations where '$i18nPolymer{msgId}' doesn't expand to '\"moo\" said the ...
5 months, 2 weeks ago (2017-02-11 01:07:16 UTC) #24
dschuyler
On 2017/02/11 01:07:16, Dan Beam wrote: > how do we end up in situations where ...
5 months, 2 weeks ago (2017-02-11 01:30:54 UTC) #25
Dan Beam (no longer on Chrome)
lgtm thanks for sticking with me on tracking down why this works
5 months, 2 weeks ago (2017-02-11 02:35:28 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2681623002/80001
5 months, 1 week ago (2017-02-13 19:28:20 UTC) #28
commit-bot: I haz the power
Failed to apply patch for chrome/browser/resources/settings/site_settings_page/site_settings_page.html: While running git apply --index -p1; error: patch failed: ...
5 months, 1 week ago (2017-02-13 21:09:41 UTC) #30
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/2681623002/diff/100001/ui/base/template_expressions.cc File ui/base/template_expressions.cc (right): https://codereview.chromium.org/2681623002/diff/100001/ui/base/template_expressions.cc#newcode23 ui/base/template_expressions.cc:23: for (const auto& c : in_string) { nit: a ...
5 months, 1 week ago (2017-02-14 00:25:54 UTC) #33
dschuyler
https://codereview.chromium.org/2681623002/diff/100001/ui/base/template_expressions.cc File ui/base/template_expressions.cc (right): https://codereview.chromium.org/2681623002/diff/100001/ui/base/template_expressions.cc#newcode23 ui/base/template_expressions.cc:23: for (const auto& c : in_string) { On 2017/02/14 ...
5 months, 1 week ago (2017-02-14 00:38:35 UTC) #36
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/2681623002/diff/100001/ui/base/template_expressions.cc File ui/base/template_expressions.cc (right): https://codereview.chromium.org/2681623002/diff/100001/ui/base/template_expressions.cc#newcode23 ui/base/template_expressions.cc:23: for (const auto& c : in_string) { On 2017/02/14 ...
5 months, 1 week ago (2017-02-14 00:58:46 UTC) #37
dschuyler
https://codereview.chromium.org/2681623002/diff/100001/ui/base/template_expressions.cc File ui/base/template_expressions.cc (right): https://codereview.chromium.org/2681623002/diff/100001/ui/base/template_expressions.cc#newcode23 ui/base/template_expressions.cc:23: for (const auto& c : in_string) { On 2017/02/14 ...
5 months, 1 week ago (2017-02-14 02:30:12 UTC) #40
Dan Beam (no longer on Chrome)
still lgtm https://codereview.chromium.org/2681623002/diff/120001/ui/base/template_expressions_unittest.cc File ui/base/template_expressions_unittest.cc (right): https://codereview.chromium.org/2681623002/diff/120001/ui/base/template_expressions_unittest.cc#newcode60 ui/base/template_expressions_unittest.cc:60: substitutions["doubleSample"] = "\"mood\" said the cow"; nit: ...
5 months, 1 week ago (2017-02-14 02:34:45 UTC) #41
dschuyler
https://codereview.chromium.org/2681623002/diff/120001/ui/base/template_expressions_unittest.cc File ui/base/template_expressions_unittest.cc (right): https://codereview.chromium.org/2681623002/diff/120001/ui/base/template_expressions_unittest.cc#newcode60 ui/base/template_expressions_unittest.cc:60: substitutions["doubleSample"] = "\"mood\" said the cow"; On 2017/02/14 02:34:45, ...
5 months, 1 week ago (2017-02-14 17:59:36 UTC) #46
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2681623002/140001
5 months, 1 week ago (2017-02-14 18:00:19 UTC) #49
commit-bot: I haz the power
5 months, 1 week ago (2017-02-14 18:26:16 UTC) #52
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/0e79f6961d70e8f90eb87ecf355e...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 25c286973