Adds the ability to display text in a textfield when the text is

Adds the ability to display text in a textfield when the text is
empty.

BUG=none
TEST=none

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

[sky, 2010-03-22 21:48:33]

Elliot is OOO today. Oshima, could you take a look?

Thanks,

  -Scott

[oshima, 2010-03-22 22:12:40]

Ok, will do today.

- oshima

On 2010/03/22 21:48:33, sky wrote:
> Elliot is OOO today. Oshima, could you take a look?
> 
> Thanks,
> 
>   -Scott

[oshima, 2010-03-22 23:22:59]

please move the kPasswordChar to anonymous namespace (it's my fault), then LGTM

On 2010/03/22 22:12:40, oshima wrote:
> Ok, will do today.
> 
> - oshima
> 
> On 2010/03/22 21:48:33, sky wrote:
> > Elliot is OOO today. Oshima, could you take a look?
> > 
> > Thanks,
> > 
> >   -Scott

[oshima, 2010-03-22 23:23:27]

oops,forgot to send this. 
(still lgtm with kPasswordChar change)

http://codereview.chromium.org/1155008/diff/3001/4001
File views/controls/textfield/gtk_views_entry.cc (right):

http://codereview.chromium.org/1155008/diff/3001/4001#newcode26
views/controls/textfield/gtk_views_entry.cc:26: if (host && event->window ==
entry->text_area &&
just curious. when the above condition (with window) fails?

http://codereview.chromium.org/1155008/diff/3001/4001#newcode40
views/controls/textfield/gtk_views_entry.cc:40: gfx::Canvas::TEXT_ALIGN_LEFT);
Will this do the right thing for RTL? or just add TODO if not.

http://codereview.chromium.org/1155008/diff/3001/4003
File views/controls/textfield/native_textfield_gtk.cc (right):

http://codereview.chromium.org/1155008/diff/3001/4003#newcode20
views/controls/textfield/native_textfield_gtk.cc:20: static const char
kPasswordChar = '*';
This is my fault, but it's better to move to anonymous namespace, then we don't
need static.

http://codereview.chromium.org/1155008/diff/3001/4003#newcode252
views/controls/textfield/native_textfield_gtk.cc:252:
NativeControlCreated(textfield);
you don't need textfield, but it's up to you.

[sky, 2010-03-23 15:43:40]

New snapshot pushed.

http://codereview.chromium.org/1155008/diff/3001/4001
File views/controls/textfield/gtk_views_entry.cc (right):

http://codereview.chromium.org/1155008/diff/3001/4001#newcode26
views/controls/textfield/gtk_views_entry.cc:26: if (host && event->window ==
entry->text_area &&
On 2010/03/22 23:23:27, oshima wrote:
> just curious. when the above condition (with window) fails?

event->window == entry->text_area ?
GtkEntry internally creates a separate window (text_area). This function gets
called for both windows and I only want to paint on the one that draws the
actual text. I'll add a comment.

http://codereview.chromium.org/1155008/diff/3001/4001#newcode40
views/controls/textfield/gtk_views_entry.cc:40: gfx::Canvas::TEXT_ALIGN_LEFT);
On 2010/03/22 23:23:27, oshima wrote:
> Will this do the right thing for RTL? or just add TODO if not.

Done.

http://codereview.chromium.org/1155008/diff/3001/4003
File views/controls/textfield/native_textfield_gtk.cc (right):

http://codereview.chromium.org/1155008/diff/3001/4003#newcode20
views/controls/textfield/native_textfield_gtk.cc:20: static const char
kPasswordChar = '*';
On 2010/03/22 23:23:27, oshima wrote:
> This is my fault, but it's better to move to anonymous namespace, then we
don't
> need static.

Why is an anonymous namespace better than static?

http://codereview.chromium.org/1155008/diff/3001/4003#newcode252
views/controls/textfield/native_textfield_gtk.cc:252:
NativeControlCreated(textfield);
On 2010/03/22 23:23:27, oshima wrote:
> you don't need textfield, but it's up to you.

Done.

[oshima, 2010-03-23 16:43:24]

On Tue, Mar 23, 2010 at 8:43 AM, <sky@chromium.org> wrote:

> New snapshot pushed.
>
>
>
> http://codereview.chromium.org/1155008/diff/3001/4001
> File views/controls/textfield/gtk_views_entry.cc (right):
>
> http://codereview.chromium.org/1155008/diff/3001/4001#newcode26
> views/controls/textfield/gtk_views_entry.cc:26: if (host &&
> event->window == entry->text_area &&
> On 2010/03/22 23:23:27, oshima wrote:
>
>> just curious. when the above condition (with window) fails?
>>
>
> event->window == entry->text_area ?
> GtkEntry internally creates a separate window (text_area). This function
> gets called for both windows and I only want to paint on the one that
> draws the actual text. I'll add a comment.
>
>
> http://codereview.chromium.org/1155008/diff/3001/4001#newcode40
> views/controls/textfield/gtk_views_entry.cc:40:
> gfx::Canvas::TEXT_ALIGN_LEFT);
> On 2010/03/22 23:23:27, oshima wrote:
>
>> Will this do the right thing for RTL? or just add TODO if not.
>>
>
> Done.
>
>
> http://codereview.chromium.org/1155008/diff/3001/4003
> File views/controls/textfield/native_textfield_gtk.cc (right):
>
> http://codereview.chromium.org/1155008/diff/3001/4003#newcode20
> views/controls/textfield/native_textfield_gtk.cc:20: static const char
> kPasswordChar = '*';
> On 2010/03/22 23:23:27, oshima wrote:
>
>> This is my fault, but it's better to move to anonymous namespace, then
>>
> we don't
>
>> need static.
>>
>
> Why is an anonymous namespace better than static?
>
>
static works only for objects, but namespace works for declarations as well.
It won't make much difference in this case, but if it requires private type
as well,
you can put them in the same anonymous namespace.
It's minor, so you can keep it if you feel strongly about it.


> http://codereview.chromium.org/1155008/diff/3001/4003#newcode252
> views/controls/textfield/native_textfield_gtk.cc:252:
> NativeControlCreated(textfield);
> On 2010/03/22 23:23:27, oshima wrote:
>
>> you don't need textfield, but it's up to you.
>>
>
> Done.
>
>
> http://codereview.chromium.org/1155008
>