Add a help bubble pointing to the omnibox when installing an extension with

Add a help bubble pointing to the omnibox when installing an extension with
an omnibox keyword.

BUG=54353
TEST=Install chrome_search from the extension examples folder. You should see a help bubble pointing to the omnibox explaining how to use the omnibox keyword.

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

[Matt Perry, 2010-11-10 23:52:23]

andy: please make sure I didn't horribly screw up the Mac stuff
aa: the rest

[Aaron Boodman, 2010-11-11 02:55:30]

It would be cool if we could actually put the omnibox into keyword mode for the
extension when this bubble shows, and have the cursor in the omnibox. Possible?

http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_install_ui.cc (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_install_ui.cc:160:
!extension->page_action()->default_icon_path().empty())) {
Nit: this line needs to be indented one space.

http://codereview.chromium.org/4724005/diff/1/chrome/browser/gtk/extension_in...
File chrome/browser/gtk/extension_installed_bubble_gtk.cc (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/gtk/extension_in...
chrome/browser/gtk/extension_installed_bubble_gtk.cc:265: bounds.set_width(0);
Do you have a screenshot of what this ends up looking like?

http://codereview.chromium.org/4724005/diff/1/chrome/browser/views/extensions...
File chrome/browser/views/extensions/extension_installed_bubble.h (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/views/extensions...
chrome/browser/views/extensions/extension_installed_bubble.h:13: #endif  //
CHROME_BROWSER_VIEWS_EXTENSIONS_EXTENSION_INSTALLED_BUBBLE_H_
Remove whitespace change?

[Aaron Boodman, 2010-11-11 02:57:14]

http://codereview.chromium.org/4724005/diff/1/chrome/app/generated_resources.grd
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/4724005/diff/1/chrome/app/generated_resources....
chrome/app/generated_resources.grd:3712: Type "<ph
name="EXTENSION_KEYWORD">$1<ex>search</ex></ph>" into the URL bar to send
commands to this extension.
How about:

To use it, type "<keyword>", then TAB, then your search string.

[Bons, 2010-11-11 19:28:57]

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
File chrome/browser/cocoa/extension_installed_bubble_controller.h (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
chrome/browser/cocoa/extension_installed_bubble_controller.h:35:
kOmniboxKeyword,
while you're in here, mind alphabetizing these?

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
File chrome/browser/cocoa/extension_installed_bubble_controller.mm (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
chrome/browser/cocoa/extension_installed_bubble_controller.mm:262:
[infoBubbleView_ setArrowLocation:info_bubble::kTopLeft];
is there a default arrow position? I ask because if it defaults to be at the top
left, then you'd only have to change it if the type was not the OmniboxKeyword.

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
chrome/browser/cocoa/extension_installed_bubble_controller.mm:362: -
(NSRect)getPageActionInfoMsgFrame {
i guess this method should be renamed as well.

[Matt Perry, 2010-11-12 07:20:24]

RE: putting the omnibox in keyword mode - I still have to look into it, but I'll
keep it for a future CL regardless to avoid cluttering this one.

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
File chrome/browser/cocoa/extension_installed_bubble_controller.h (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
chrome/browser/cocoa/extension_installed_bubble_controller.h:35:
kOmniboxKeyword,
On 2010/11/11 19:28:57, Andrew Bonventre (Bons) wrote:
> while you're in here, mind alphabetizing these?

Done.

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
File chrome/browser/cocoa/extension_installed_bubble_controller.mm (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
chrome/browser/cocoa/extension_installed_bubble_controller.mm:262:
[infoBubbleView_ setArrowLocation:info_bubble::kTopLeft];
On 2010/11/11 19:28:57, Andrew Bonventre (Bons) wrote:
> is there a default arrow position? I ask because if it defaults to be at the
top
> left, then you'd only have to change it if the type was not the
OmniboxKeyword.

There is, but it feels safer not relying on the defaults to be a certain way.

http://codereview.chromium.org/4724005/diff/1/chrome/browser/cocoa/extension_...
chrome/browser/cocoa/extension_installed_bubble_controller.mm:362: -
(NSRect)getPageActionInfoMsgFrame {
On 2010/11/11 19:28:57, Andrew Bonventre (Bons) wrote:
> i guess this method should be renamed as well.

Done.

http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_install_ui.cc (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_install_ui.cc:160:
!extension->page_action()->default_icon_path().empty())) {
On 2010/11/11 02:55:30, Aaron Boodman wrote:
> Nit: this line needs to be indented one space.

Are you sure? I don't think I've ever seen a multiply-parenthesized expression
formatted that way.

http://codereview.chromium.org/4724005/diff/1/chrome/browser/gtk/extension_in...
File chrome/browser/gtk/extension_installed_bubble_gtk.cc (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/gtk/extension_in...
chrome/browser/gtk/extension_installed_bubble_gtk.cc:265: bounds.set_width(0);
On 2010/11/11 02:55:30, Aaron Boodman wrote:
> Do you have a screenshot of what this ends up looking like?

I attached screenshots of the Windows UI to the bug, but in the process
discovered RTL is broken under linux. Working on a fix.

[Aaron Boodman, 2010-11-12 19:04:05]

http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_install_ui.cc (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_install_ui.cc:160:
!extension->page_action()->default_icon_path().empty())) {
On 2010/11/12 07:20:25, Matt Perry wrote:
> On 2010/11/11 02:55:30, Aaron Boodman wrote:
> > Nit: this line needs to be indented one space.
> 
> Are you sure? I don't think I've ever seen a multiply-parenthesized expression
> formatted that way.

The expression "!extension->page_action()->default_icon_path().empty()" is a
child of the left paren above the exclamation point. Typically don't we do:

if (very_long_expression &&
    other_long_expression) {

}

?

[Matt Perry, 2010-11-12 23:08:18]

OK, I fixed the linux RTL bug. I think this is ready for commit once you guys
approve.

http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_install_ui.cc (right):

http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_install_ui.cc:160:
!extension->page_action()->default_icon_path().empty())) {
On 2010/11/12 19:04:06, Aaron Boodman wrote:
> On 2010/11/12 07:20:25, Matt Perry wrote:
> > On 2010/11/11 02:55:30, Aaron Boodman wrote:
> > > Nit: this line needs to be indented one space.
> > 
> > Are you sure? I don't think I've ever seen a multiply-parenthesized
expression
> > formatted that way.
> 
> The expression "!extension->page_action()->default_icon_path().empty()" is a
> child of the left paren above the exclamation point. Typically don't we do:
> 
> if (very_long_expression &&
>     other_long_expression) {
> 
> }
> 
> ?

Yes, but I've only seen it stop there. It looks weird to me to do:

if (expr1 &&
    (expr2 &&
     expr3) &&
    expr4) {
}

but there's nothing in the style guide either way.

[Bons, 2010-11-12 23:54:04]

Did you work out your domestic issues with interface builder?

On Fri, Nov 12, 2010 at 6:08 PM,  <mpcomplete@chromium.org> wrote:
> OK, I fixed the linux RTL bug. I think this is ready for commit once you
> guys
> approve.
>
>
>
http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
> File chrome/browser/extensions/extension_install_ui.cc (right):
>
>
http://codereview.chromium.org/4724005/diff/1/chrome/browser/extensions/exten...
> chrome/browser/extensions/extension_install_ui.cc:160:
> !extension->page_action()->default_icon_path().empty())) {
> On 2010/11/12 19:04:06, Aaron Boodman wrote:
>>
>> On 2010/11/12 07:20:25, Matt Perry wrote:
>> > On 2010/11/11 02:55:30, Aaron Boodman wrote:
>> > > Nit: this line needs to be indented one space.
>> >
>> > Are you sure? I don't think I've ever seen a multiply-parenthesized
>
> expression
>>
>> > formatted that way.
>
>> The expression
>
> "!extension->page_action()->default_icon_path().empty()" is a
>>
>> child of the left paren above the exclamation point. Typically don't
>
> we do:
>
>> if (very_long_expression &&
>>     other_long_expression) {
>
>> }
>
>> ?
>
> Yes, but I've only seen it stop there. It looks weird to me to do:
>
> if (expr1 &&
>    (expr2 &&
>     expr3) &&
>    expr4) {
> }
>
> but there's nothing in the style guide either way.
>
> http://codereview.chromium.org/4724005/
>

[Matt Perry, 2010-11-12 23:57:05]

On 2010/11/12 23:54:04, Andrew Bonventre (Bons) wrote:
> Did you work out your domestic issues with interface builder?

No, we had irreconcilable differences. Can you can patch this CL into your
checkout and fix up the .xib for me?

[Bons, 2010-11-12 23:59:28]

I'm sorry to hear that. I'd be happy to, though I probably won't get
to it 'til tomorrow. That cool?

On Fri, Nov 12, 2010 at 6:57 PM,  <mpcomplete@chromium.org> wrote:
> On 2010/11/12 23:54:04, Andrew Bonventre (Bons) wrote:
>>
>> Did you work out your domestic issues with interface builder?
>
> No, we had irreconcilable differences. Can you can patch this CL into your
> checkout and fix up the .xib for me?
>
> http://codereview.chromium.org/4724005/
>

[Matt Perry, 2010-11-13 00:01:39]

Yeah that's fine. Feel free to wait till Monday, actually :).

On 2010/11/12 23:59:28, Andrew Bonventre (Bons) wrote:
> I'm sorry to hear that. I'd be happy to, though I probably won't get
> to it 'til tomorrow. That cool?
> 
> On Fri, Nov 12, 2010 at 6:57 PM,  <mailto:mpcomplete@chromium.org> wrote:
> > On 2010/11/12 23:54:04, Andrew Bonventre (Bons) wrote:
> >>
> >> Did you work out your domestic issues with interface builder?
> >
> > No, we had irreconcilable differences. Can you can patch this CL into your
> > checkout and fix up the .xib for me?
> >
> > http://codereview.chromium.org/4724005/
> >