Chromium Code Reviews
Help | Chromium Project | Sign in
(820)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by Nebojša Ćirić
Modified:
2 years, 11 months ago
Reviewers:
Jungshik Shin, rafaelw
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) Lint Patch
M chrome/common/extensions/extension_constants.h View 1 chunk +1 line, -0 lines 0 comments 0 errors Download
M chrome/common/extensions/extension_constants.cc View 1 chunk +2 lines, -0 lines 0 comments 0 errors Download
M chrome/common/extensions/extension_message_bundle.h View 1 2 3 4 chunks +28 lines, -6 lines 2 comments 0 errors Download
M chrome/common/extensions/extension_message_bundle.cc View 1 2 3 4 chunks +59 lines, -1 line 0 comments 0 errors Download
M chrome/common/extensions/extension_message_bundle_unittest.cc View 1 2 3 3 chunks +253 lines, -151 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 8
Nebojša Ćirić
First item on my todo list after beta launch. Use cases would be: 1. Makes ...
4 years, 3 months ago #1
Nebojša Ćirić
ping (added 3 more reserved messages - see description for details).
4 years, 3 months ago #2
Nebojša Ćirić
Added checks (and error message) to see if we overwrote any of the user messages ...
4 years, 3 months ago #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: ...
4 years, 3 months ago #4
Jungshik Shin
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 = ...
4 years, 3 months ago #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 ...
4 years, 3 months ago #6
Jungshik Shin
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: ...
4 years, 3 months ago #7
Nebojša Ćirić
4 years, 3 months ago #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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6