Polished on side-loaded extension install alert.

Polished on side-loaded extension install alert.

BUG=111674
TEST=Install 2 external extensions. Check popup bubble on installation.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124874

[chebert, 2012-02-22 03:49:58]

Please review my first CL:
Polish on side-loaded extension install alert

[chebert, 2012-02-23 23:40:18]

Updated CL for Polished side-loaded extension alert bubble.

https://chromiumcodereview.appspot.com/9372091/diff/1/chrome/app/chromium_str...
File chrome/app/chromium_strings.grd (right):

https://chromiumcodereview.appspot.com/9372091/diff/1/chrome/app/chromium_str...
chrome/app/chromium_strings.grd:613: <ph name="EXTENSION_NAME">$1<ex>Gmail
Checker</ex></ph> has been added.
Why did you do this here, but not in the google_chrome_strings.grd? Make a
decision but be consistent.

[clintstaley, 2012-02-24 02:21:16]

LGTM

[miket_OOO, 2012-02-24 17:16:52]

LGTM. Note that I am not a Chromium committer, so my approval is neither
required nor meaningful. Crawl up the OWNERS chain to find a suitable reviewer.

[Aaron Boodman, 2012-02-28 22:55:16]

Please provide a screenshot of the new behavior... you can upload to imgur.com.

[chebert, 2012-02-28 23:21:32]

On 2012/02/28 22:55:16, Aaron Boodman wrote:
> Please provide a screenshot of the new behavior... you can upload to
http://imgur.com.

[IMG]http://i42.tinypic.com/2u5vo69.png[/IMG]

[Aaron Boodman, 2012-02-29 01:39:34]

We now have a double-line-break after the last entry. A hacky way to fix this
would be to just remove the last character from the text if it is a newline.

[Aaron Boodman, 2012-02-29 01:39:57]

http://codereview.chromium.org/9372091/diff/3001/chrome/app/chromium_strings.grd
File chrome/app/chromium_strings.grd (right):

http://codereview.chromium.org/9372091/diff/3001/chrome/app/chromium_strings....
chrome/app/chromium_strings.grd:608: <message
name="IDS_EXTENSION_ALERT_ITEM_EXTERNAL" desc="A statement that an external
extension has been newly installed. End users have no idea what an 'external'
extension is, so we simply call them extensions.">
It would be good to note here that the trailing character is expected to be a
newline.

[chebert, 2012-03-02 01:39:13]

Here is an updated screenshot:
http://imgur.com/JdlY2

[chebert, 2012-03-02 01:58:36]

https://chromiumcodereview.appspot.com/9372091/diff/3001/chrome/app/chromium_...
File chrome/app/chromium_strings.grd (right):

https://chromiumcodereview.appspot.com/9372091/diff/3001/chrome/app/chromium_...
chrome/app/chromium_strings.grd:608: <message
name="IDS_EXTENSION_ALERT_ITEM_EXTERNAL" desc="A statement that an external
extension has been newly installed. End users have no idea what an 'external'
extension is, so we simply call them extensions.">
On 2012/02/29 01:39:57, Aaron Boodman wrote:
> It would be good to note here that the trailing character is expected to be a
> newline.

Done.

[Aaron Boodman, 2012-03-02 18:47:35]

LGTM

Please correct the two minor comments before landing.

https://chromiumcodereview.appspot.com/9372091/diff/15001/chrome/app/chromium...
File chrome/app/chromium_strings.grd (right):

https://chromiumcodereview.appspot.com/9372091/diff/15001/chrome/app/chromium...
chrome/app/chromium_strings.grd:608: <message
name="IDS_EXTENSION_ALERT_ITEM_EXTERNAL" desc="A statement that an external
extension has been newly installed. End users have no idea what an 'external'
extension is, so we simply call them extensions. The trailing character is
expected to be a newline.">
Sorry to nitpick here, but can you change this to:

NOTE: The last character must be a newline.

These files get translated by non-programmers. The message in "desc" gets
presented to them in a UI. Any odd expectations must be carefully called out
there.

https://chromiumcodereview.appspot.com/9372091/diff/15001/chrome/browser/exte...
File chrome/browser/extensions/extension_global_error.cc (right):

https://chromiumcodereview.appspot.com/9372091/diff/15001/chrome/browser/exte...
chrome/browser/extensions/extension_global_error.cc:97: if
(message_[message_.size()-1] == '\n') {
No curly brace needed for two-line if statements.

[commit-bot: I haz the power, 2012-03-03 18:25:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hebert.christopherj@chromium.org/93720...

[commit-bot: I haz the power, 2012-03-03 19:54:38]

Change committed as 124874

[Evan Stade, 2012-03-19 21:45:56]

http://codereview.chromium.org/9372091/diff/22001/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_global_error.cc (right):

http://codereview.chromium.org/9372091/diff/22001/chrome/browser/extensions/e...
chrome/browser/extensions/extension_global_error.cc:98:
message_.resize(message_.size()-1);
spaces around operators.

[Aaron Boodman, 2012-03-19 22:00:07]

http://codereview.chromium.org/9372091/diff/22001/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_global_error.cc (right):

http://codereview.chromium.org/9372091/diff/22001/chrome/browser/extensions/e...
chrome/browser/extensions/extension_global_error.cc:98:
message_.resize(message_.size()-1);
On 2012/03/19 21:45:56, Evan Stade wrote:
> spaces around operators.

Dang, how did I miss this... Thanks Evan.

[chebert, 2012-03-26 22:22:24]

http://codereview.chromium.org/9372091/diff/22001/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_global_error.cc (right):

http://codereview.chromium.org/9372091/diff/22001/chrome/browser/extensions/e...
chrome/browser/extensions/extension_global_error.cc:98:
message_.resize(message_.size()-1);
On 2012/03/19 21:45:56, Evan Stade wrote:
> spaces around operators.

Done.