Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(556)

Issue 546040: Add reserved messages to ExtensionMessageBundle dictionary. They are of the f... (Closed)

Created:
10 years, 11 months ago by Nebojša Ćirić
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com, Aaron Boodman, Erik does not do reviews, pam+watch_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

Add reserved messages to ExtensionMessageBundle dictionary. They are of the form @@somename, i.e. @@ui_locale. It makes easier for developers to detect current UI locale (also available though window.navigator.language), or text direction. I'll use them for static message replacement later on too. Before this change developers would have to manualy add locale and text_dir messages and translate for all supported locales. Added 5 reserved messages: @@ui_locale @@bidi_dir @@bidi_reversed_dir @@bidi_start_edge @@bidi_end_edge See http://code.google.com/apis/gadgets/docs/i18n.html#BIDI on why are they usefull. Extended allowed charset for variable names with @. Added unittest for variable names. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=36693

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 6

Patch Set 3 : '' #

Total comments: 8

Patch Set 4 : '' #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+343 lines, -158 lines) Patch
M chrome/common/extensions/extension_constants.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/extensions/extension_constants.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/extensions/extension_message_bundle.h View 1 2 3 4 chunks +28 lines, -6 lines 2 comments Download
M chrome/common/extensions/extension_message_bundle.cc View 1 2 3 4 chunks +59 lines, -1 line 0 comments Download
M chrome/common/extensions/extension_message_bundle_unittest.cc View 1 2 3 3 chunks +253 lines, -151 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Nebojša Ćirić
First item on my todo list after beta launch. Use cases would be: 1. Makes ...
10 years, 11 months ago (2010-01-13 22:30:22 UTC) #1
Nebojša Ćirić
ping (added 3 more reserved messages - see description for details).
10 years, 11 months ago (2010-01-15 22:43:09 UTC) #2
Nebojša Ćirić
Added checks (and error message) to see if we overwrote any of the user messages ...
10 years, 11 months ago (2010-01-19 22:13:41 UTC) #3
rafaelw
lgtm w/ nits. http://codereview.chromium.org/546040/diff/7003/7004 File chrome/common/extensions/extension_message_bundle.cc (right): http://codereview.chromium.org/546040/diff/7003/7004#newcode92 chrome/common/extensions/extension_message_bundle.cc:92: nit; remove this newline. http://codereview.chromium.org/546040/diff/7003/7004#newcode114 chrome/common/extensions/extension_message_bundle.cc:114: ...
10 years, 11 months ago (2010-01-20 01:27:34 UTC) #4
jungshik at Google
lgtm with the following comments. http://codereview.chromium.org/546040/diff/4002/5001 File chrome/common/extensions/extension_message_bundle.cc (right): http://codereview.chromium.org/546040/diff/4002/5001#newcode28 chrome/common/extensions/extension_message_bundle.cc:28: const char* ExtensionMessageBundle::kUILocaleKey = ...
10 years, 11 months ago (2010-01-20 19:10:05 UTC) #5
Nebojša Ćirić
Fixed all. Rafael, I've edited extension_constants to add my new error message to it. http://codereview.chromium.org/546040/diff/4002/5001 ...
10 years, 11 months ago (2010-01-20 19:42:12 UTC) #6
jungshik at Google
lgtm http://codereview.chromium.org/546040/diff/4005/4007 File chrome/common/extensions/extension_message_bundle.h (right): http://codereview.chromium.org/546040/diff/4005/4007#newcode102 chrome/common/extensions/extension_message_bundle.h:102: // Appends reserved messages to the dictionary. nit: ...
10 years, 11 months ago (2010-01-20 21:35:47 UTC) #7
Nebojša Ćirić
10 years, 11 months ago (2010-01-20 22:40:21 UTC) #8
http://codereview.chromium.org/546040/diff/4005/4007
File chrome/common/extensions/extension_message_bundle.h (right):

http://codereview.chromium.org/546040/diff/4005/4007#newcode102
chrome/common/extensions/extension_message_bundle.h:102: // Appends reserved
messages to the dictionary.
On 2010/01/20 21:35:47, Jungshik Shin wrote:
> nit: "Appends locale-specific reserved messages to the dictionary" or 'Appends
> reserved msgs for |locale| to the dictionary' is better? Should we change the
> function name to 'AppendReservedMessageForLocale' as well? All the reserved
> messages are locale-specific at the moment. So, it's ok to leave the name
alone
> (although at call sites, it can be a bit confusing). 

Done.

Powered by Google App Engine
This is Rietveld 408576698