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

Issue 13896002: Tweak Views Omnibox/Textfield; re-enable OmniboxViewTest.SelectAllOnClick. (Closed)

Created:
7 years, 8 months ago by msw
Modified:
7 years, 8 months ago
Reviewers:
Daniel Erat, sky
CC:
chromium-reviews, Cem Kocagil
Visibility:
Public.

Description

Tweak Views Omnibox/Textfield; re-enable OmniboxViewTest.SelectAllOnClick. Update test expectations for OmniboxViewTest.SelectAllOnClick: (CrOS used to show/retain unfocused (grey highlight) Omnibox selections) (My http://crrev.com/183239 made it save/restore selections like Windows) Don't expect unfocused selections; check that clicking anew selects all. Move the base omnibox click offset over to the right a little. (fixes the test for OmniboxViewWin's slightly different hit-test area) Enable previously USE_AURA-limited code on all TOOLKIT_VIEWS. Update NativeTextfieldViews::OnMouseDragged to only handle left-click. (it was incorrectly and incompletely handling right- and middle-drag) Make NativeTextfieldViews::HandleMousePressEvent focus on right-click. (it was selecting all and showing the context menu without taking focus) Use ui_test_utils::IsViewFocused() instead of GetFocusView()->HasFocus(); Inline the last remaining GetFocusView call; some refactoring. BUG=128556, 139069 TEST=None; OmniboxViewTest.SelectAllOnClick passes. R=derat@chromium.org,sky@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193404

Patch Set 1 #

Patch Set 2 : Inline the last use of GetFocusView; cleanup. #

Patch Set 3 : Remove extra click causing hangs on Win; rename ClickOmnibox. #

Patch Set 4 : Fix click location; enable for TOOLKIT_VIEWS; fix Views Textfields middle/right-click. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+64 lines, -80 lines) Patch
M chrome/browser/ui/omnibox/omnibox_view_browsertest.cc View 1 2 3 4 chunks +36 lines, -56 lines 0 comments Download
M ui/views/controls/textfield/native_textfield_views.cc View 1 2 3 2 chunks +28 lines, -24 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
msw
Hey Dan and Scott, please take a look; thanks! Note Cem's similar CL: https://codereview.chromium.org/13084017/ (he ...
7 years, 8 months ago (2013-04-10 01:08:09 UTC) #1
Daniel Erat
lgtm
7 years, 8 months ago (2013-04-10 04:03:37 UTC) #2
sky
LGTM
7 years, 8 months ago (2013-04-10 14:06:40 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/13896002/20001
7 years, 8 months ago (2013-04-10 14:47:00 UTC) #4
commit-bot: I haz the power
7 years, 8 months ago (2013-04-10 17:17:31 UTC) #5
Message was sent while issue was closed.
Change committed as 193404

Powered by Google App Engine
This is Rietveld 408576698