Identify the omnibox as a URL field.

Identify the omnibox as a URL field.

Although the omnibox is not strictly a URL field, for the purposes of
TOUCH_UI, we would prefer to show the URL-friendly keyboard for the
omnibox instead of a plain-text keyboard.

BUG=none
TEST=updated unittest


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

[oshima, 2011-09-02 20:54:01]

http://codereview.chromium.org/7826039/diff/1/views/controls/textfield/textfi...
File views/controls/textfield/textfield.h (right):

http://codereview.chromium.org/7826039/diff/1/views/controls/textfield/textfi...
views/controls/textfield/textfield.h:54: STYLE_URL       = 1 << 2
I'm a bit worried about expanding this for input type.
How about adding TextInputType in Textfield and add settter/getter? (with
default based on StyleFlags) 
That way, when you need to support more type,
you can just add it TextInputType without changing Textfield class. What do you
think?

[bryeung, 2011-09-02 21:28:38]

PTAL

I'm not entirely happy with how I've connected SetPassword and SetInputTextType
together: suggestions welcome :-)

[oshima, 2011-09-02 22:00:10]

please add TODO and moved the method (see my comments below), then LGTM

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/nat...
File views/controls/textfield/native_textfield_views.cc (right):

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/nat...
views/controls/textfield/native_textfield_views.cc:702: ui::TextInputType
NativeTextfieldViews::GetTextInputType() {
looks like we can move this to Textfield?

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/tex...
File views/controls/textfield/textfield.cc (right):

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/tex...
views/controls/textfield/textfield.cc:113: }
I think the only place this is used is in
chrome/browser/chromeos/options/wifi_config_view.cc, which we will replace with
webui. Could you please add TODO to revisit and remove if this is no longer
necessary?

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/tex...
views/controls/textfield/textfield.cc:125: bool should_be_password = type ==
ui::TEXT_INPUT_TYPE_PASSWORD;
This is fine with me, or we can reject TEXT_INPUT_TYPE_PASSWORD to avoid misuse.
I'll leave it to you.

[bryeung, 2011-09-02 23:33:11]

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/nat...
File views/controls/textfield/native_textfield_views.cc (right):

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/nat...
views/controls/textfield/native_textfield_views.cc:702: ui::TextInputType
NativeTextfieldViews::GetTextInputType() {
On 2011/09/02 22:00:10, oshima wrote:
> looks like we can move this to Textfield?

It's actually an override from TextInputClient (which Textfield isn't).

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/tex...
File views/controls/textfield/textfield.cc (right):

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/tex...
views/controls/textfield/textfield.cc:113: }
On 2011/09/02 22:00:10, oshima wrote:
> I think the only place this is used is in
> chrome/browser/chromeos/options/wifi_config_view.cc, which we will replace
with
> webui. Could you please add TODO to revisit and remove if this is no longer
> necessary?

Done.

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/tex...
views/controls/textfield/textfield.cc:125: bool should_be_password = type ==
ui::TEXT_INPUT_TYPE_PASSWORD;
On 2011/09/02 22:00:10, oshima wrote:
> This is fine with me, or we can reject TEXT_INPUT_TYPE_PASSWORD to avoid
misuse.
> I'll leave it to you.

Okay: I'll leave it like this for now and it will get cleaned up when
SetPassword goes away.

[bryeung, 2011-09-02 23:34:23]

Sadrul: Oshima has reviewed but I need an LGTM from an OWNER.  Can you have a
look?

[sadrul, 2011-09-02 23:47:37]

LGTM

http://codereview.chromium.org/7826039/diff/5003/views/controls/textfield/tex...
File views/controls/textfield/textfield.cc (right):

http://codereview.chromium.org/7826039/diff/5003/views/controls/textfield/tex...
views/controls/textfield/textfield.cc:119: ui::TextInputType
Textfield::GetTextInputType() const {
This could be Textfield::text_input_type() const {

[oshima, 2011-09-03 00:26:32]

LGTM

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/nat...
File views/controls/textfield/native_textfield_views.cc (right):

http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/nat...
views/controls/textfield/native_textfield_views.cc:702: ui::TextInputType
NativeTextfieldViews::GetTextInputType() {
On 2011/09/02 23:33:11, bryeung wrote:
> On 2011/09/02 22:00:10, oshima wrote:
> > looks like we can move this to Textfield?
> 
> It's actually an override from TextInputClient (which Textfield isn't).

I see, then just move the following to Textfield.

if (textfield_->read_only() || !textfield_->IsEnabled())
  return ui::TEXT_INPUT_TYPE_NONE;

http://codereview.chromium.org/7826039/diff/5003/views/controls/textfield/tex...
File views/controls/textfield/textfield.cc (right):

http://codereview.chromium.org/7826039/diff/5003/views/controls/textfield/tex...
views/controls/textfield/textfield.cc:119: ui::TextInputType
Textfield::GetTextInputType() const {
On 2011/09/02 23:47:37, sadrul wrote:
> This could be Textfield::text_input_type() const {

Textfield and NativeTextfieldViews will be merged, in which case, this should be
GetTextInputType(), so this is fine.

[bryeung, 2011-09-03 00:43:16]

Moved the code and added a few more tests for those conditions.

[commit-bot: I haz the power, 2011-09-06 14:55:18]

Change committed as 99737