Minimal SetInitialFocus and RestoreFocus. These aren't final, but this does r...

Minimal SetInitialFocus and RestoreFocus. These aren't final, but this does resolve the
location bar not being focused bug on the mac build. 

BUG=9419
Committed in http://src.chromium.org/viewvc/chrome?view=rev&revision=14939

[Mohamed Mansour (USE mhm), 2009-04-29 00:17:58]

http://codereview.chromium.org/100140/diff/1/3
File AUTHORS (right):

http://codereview.chromium.org/100140/diff/1/3#newcode8
Line 8: Paweध‚ Hajdan jr <phajdan.jr@gmail.com>
You messed up his name :)

http://codereview.chromium.org/100140/diff/1/3#newcode34
Line 34: Josu़ Ratelle <jorat1346@gmail.com>
and this too !

http://codereview.chromium.org/100140/diff/1/2
File chrome/browser/tab_contents/tab_contents_view_mac.mm (right):

http://codereview.chromium.org/100140/diff/1/2#newcode121
Line 121: SetInitialFocus(); //This will make sure the location bar is selected
if we're a new tab
nit: Put this comment above the SetInitialFocus() and make sure it is 80 char
length.
http://google-styleguide.googlecode.com/svn/trunk/objcguide.xml#Line_Length

[rohitrao (ping after 24h), 2009-04-29 00:19:09]

http://codereview.chromium.org/100140/diff/1/3
File AUTHORS (right):

http://codereview.chromium.org/100140/diff/1/3#newcode8
Line 8: Paweध‚ Hajdan jr <phajdan.jr@gmail.com>
Not sure why these characters are getting munged.  Does your editor handle UTF-8
(?) correctly?

http://codereview.chromium.org/100140/diff/1/2
File chrome/browser/tab_contents/tab_contents_view_mac.mm (right):

http://codereview.chromium.org/100140/diff/1/2#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
Change 2006-2008 to 2009.

http://codereview.chromium.org/100140/diff/1/2#newcode110
Line 110: // TODO(port)
Change this to something like:
  // TODO(port): Set focus correctly in the else case.

http://codereview.chromium.org/100140/diff/1/2#newcode121
Line 121: SetInitialFocus(); //This will make sure the location bar is selected
if we're a new tab
Keep lines under 80 characters.

The comment, while correct, doesn't tell the whole story.  Copying the GTK
comment for now will work.

[sbryant, 2009-04-29 12:52:13]

On 2009/04/29 00:19:09, rohitrao wrote:
> http://codereview.chromium.org/100140/diff/1/3
> File AUTHORS (right):
> 
> http://codereview.chromium.org/100140/diff/1/3#newcode8
> Line 8: Paweध‚ Hajdan jr <phajdan.jr@gmail.com>
> Not sure why these characters are getting munged.  Does your editor handle
UTF-8
> (?) correctly?
My editors seem to be screwing with the file encoding. I used 3 different
editors (emacs (saving as utf-8-auto, TextEdit, and XCode's internal editor) and
they all screwed up the name. Is UTF-8 the proper encoding? 
> 
> http://codereview.chromium.org/100140/diff/1/2
> File chrome/browser/tab_contents/tab_contents_view_mac.mm (right):
> 
> http://codereview.chromium.org/100140/diff/1/2#newcode1
> Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
> Change 2006-2008 to 2009.
> 
> http://codereview.chromium.org/100140/diff/1/2#newcode110
> Line 110: // TODO(port)
> Change this to something like:
>   // TODO(port): Set focus correctly in the else case.
> 
> http://codereview.chromium.org/100140/diff/1/2#newcode121
> Line 121: SetInitialFocus(); //This will make sure the location bar is
selected
> if we're a new tab
> Keep lines under 80 characters.
>
Done 
> The comment, while correct, doesn't tell the whole story.  Copying the GTK
> comment for now will work.
And done.

[rohitrao (ping after 24h), 2009-04-29 15:51:44]

http://codereview.chromium.org/100140/diff/8/9
File AUTHORS (right):

http://codereview.chromium.org/100140/diff/8/9#newcode37
Line 37: Sean Bryant <sean@cyberwang.net>
I think one of these lines may not be valid UTF-8, so TextEdit is getting
confused.  Maybe just "cat >> AUTHORS" and append your line to the file that
way?

[sbryant, 2009-04-29 15:58:36]

On 2009/04/29 15:51:44, rohitrao wrote:
> http://codereview.chromium.org/100140/diff/8/9
> File AUTHORS (right):
> 
> http://codereview.chromium.org/100140/diff/8/9#newcode37
> Line 37: Sean Bryant <sean@cyberwang.net>
> I think one of these lines may not be valid UTF-8, so TextEdit is getting
> confused.  Maybe just "cat >> AUTHORS" and append your line to the file that
> way?
Sure thing. Maybe we can file a bug report on this ;)

[pink (ping after 24hrs), 2009-04-29 16:41:00]

LGTM, not sure about the AUTHORs patch.

http://codereview.chromium.org/100140/diff/8/10
File chrome/browser/tab_contents/tab_contents_view_mac.mm (right):

http://codereview.chromium.org/100140/diff/8/10#newcode111
Line 111: if(web_contents()->FocusLocationBarByDefault())
space after |if|

[Avi (use Gerrit), 2009-04-29 16:48:49]

Looks quite reasonable. LG.

http://codereview.chromium.org/100140/diff/8/10
File chrome/browser/tab_contents/tab_contents_view_mac.mm (right):

http://codereview.chromium.org/100140/diff/8/10#newcode110
Line 110: // TODO(port) Set focus in else case correctly
Colon, please. TODO(insertnamehere): do this or that

[rohitrao (ping after 24h), 2009-04-29 22:15:22]

LGTM, other than the two nits.

If you upload a new patch, I'll take care of landing this.

http://codereview.chromium.org/100140/diff/16/18
File AUTHORS (right):

http://codereview.chromium.org/100140/diff/16/18#newcode37
Line 37: Sean Bryant <sean@cyberwang.net>
Files should end in a newline.

http://codereview.chromium.org/100140/diff/16/17
File chrome/browser/tab_contents/tab_contents_view_mac.mm (right):

http://codereview.chromium.org/100140/diff/16/17#newcode110
Line 110: // TODO(port): Set focus in else case correctly
Nit: Comments should have proper punctuation =)

[sbryant, 2009-04-29 23:03:58]

On 2009/04/29 22:15:22, rohitrao wrote:
> LGTM, other than the two nits.
> 
> If you upload a new patch, I'll take care of landing this.
> 
> http://codereview.chromium.org/100140/diff/16/18
> File AUTHORS (right):
> 
> http://codereview.chromium.org/100140/diff/16/18#newcode37
> Line 37: Sean Bryant <sean@cyberwang.net>
> Files should end in a newline.
Done.
> 
> http://codereview.chromium.org/100140/diff/16/17
> File chrome/browser/tab_contents/tab_contents_view_mac.mm (right):
> 
> http://codereview.chromium.org/100140/diff/16/17#newcode110
> Line 110: // TODO(port): Set focus in else case correctly
> Nit: Comments should have proper punctuation =)
Added the punctuation. I'll get the hang of it eventually ;)

[rohitrao (ping after 24h), 2009-04-30 14:56:45]

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