Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(86)

Issue 7826039: Identify the omnibox as a URL field. (Closed)

Created:
9 years, 3 months ago by bryeung
Modified:
9 years, 3 months ago
CC:
chromium-reviews, Paweł Hajdan Jr., dhollowa
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 1

Patch Set 2 : refactor based on comments #

Total comments: 7

Patch Set 3 : review comments #

Total comments: 2

Patch Set 4 : review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -9 lines) Patch
M chrome/browser/ui/views/omnibox/omnibox_view_views.cc View 1 2 chunks +2 lines, -0 lines 0 comments Download
M views/controls/textfield/native_textfield_views.cc View 1 2 3 1 chunk +1 line, -5 lines 0 comments Download
M views/controls/textfield/native_textfield_views_unittest.cc View 1 2 3 3 chunks +54 lines, -0 lines 0 comments Download
M views/controls/textfield/textfield.h View 1 2 3 chunks +11 lines, -0 lines 0 comments Download
M views/controls/textfield/textfield.cc View 1 2 3 4 chunks +26 lines, -4 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
bryeung
9 years, 3 months ago (2011-09-02 20:41:37 UTC) #1
oshima
http://codereview.chromium.org/7826039/diff/1/views/controls/textfield/textfield.h File views/controls/textfield/textfield.h (right): http://codereview.chromium.org/7826039/diff/1/views/controls/textfield/textfield.h#newcode54 views/controls/textfield/textfield.h:54: STYLE_URL = 1 << 2 I'm a bit worried ...
9 years, 3 months ago (2011-09-02 20:54:01 UTC) #2
bryeung
PTAL I'm not entirely happy with how I've connected SetPassword and SetInputTextType together: suggestions welcome ...
9 years, 3 months ago (2011-09-02 21:28:38 UTC) #3
oshima
please add TODO and moved the method (see my comments below), then LGTM http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/native_textfield_views.cc File ...
9 years, 3 months ago (2011-09-02 22:00:10 UTC) #4
bryeung
http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/native_textfield_views.cc File views/controls/textfield/native_textfield_views.cc (right): http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/native_textfield_views.cc#newcode702 views/controls/textfield/native_textfield_views.cc:702: ui::TextInputType NativeTextfieldViews::GetTextInputType() { On 2011/09/02 22:00:10, oshima wrote: > ...
9 years, 3 months ago (2011-09-02 23:33:11 UTC) #5
bryeung
Sadrul: Oshima has reviewed but I need an LGTM from an OWNER. Can you have ...
9 years, 3 months ago (2011-09-02 23:34:23 UTC) #6
sadrul
LGTM http://codereview.chromium.org/7826039/diff/5003/views/controls/textfield/textfield.cc File views/controls/textfield/textfield.cc (right): http://codereview.chromium.org/7826039/diff/5003/views/controls/textfield/textfield.cc#newcode119 views/controls/textfield/textfield.cc:119: ui::TextInputType Textfield::GetTextInputType() const { This could be Textfield::text_input_type() ...
9 years, 3 months ago (2011-09-02 23:47:37 UTC) #7
oshima
LGTM http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/native_textfield_views.cc File views/controls/textfield/native_textfield_views.cc (right): http://codereview.chromium.org/7826039/diff/5001/views/controls/textfield/native_textfield_views.cc#newcode702 views/controls/textfield/native_textfield_views.cc:702: ui::TextInputType NativeTextfieldViews::GetTextInputType() { On 2011/09/02 23:33:11, bryeung wrote: ...
9 years, 3 months ago (2011-09-03 00:26:32 UTC) #8
bryeung
Moved the code and added a few more tests for those conditions.
9 years, 3 months ago (2011-09-03 00:43:16 UTC) #9
commit-bot: I haz the power
9 years, 3 months ago (2011-09-06 14:55:18 UTC) #10
Change committed as 99737

Powered by Google App Engine
This is Rietveld 408576698