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

Issue 23710009: Revert 219673 "Fix Views Combobox and Tree View text input." (Closed)

Created:
7 years, 3 months ago by Shrikant Kelkar
Modified:
7 years, 3 months ago
Reviewers:
msw
CC:
chromium-reviews
Visibility:
Public.

Description

Revert 219673 "Fix Views Combobox and Tree View text input." Reverting because of most probable regression (https://code.google.com/p/chromium/issues/detail?id=280605). Note: We need to implement gettextinputclient in the webview class, which needs to get client from web contents. > Fix Views Combobox and Tree View text input. > > Make InputMethodBridge not a TextInputClient, use actual clients. > InputMethodBridge was simply forwarding to actual client views. > (e.g. NativeTextfieldViews and PrefixSelector for TreeView/Combobox) > > Remove Views::InputMethod::On[Focus|Blur]() and their calls. > Simply update the host's TextInputClient on View focus changes. > Fixes r216992's regression (input not sent to Combobox/TreeView). > The IME mode indicator also now shows in dialogs more often :) > > Ignore Widget activation in InputMethodBase::GetTextInputClient. > (fixes linux_chromeos initial omnibox input with this patch, seems ok) > > Call View::On[Focus|Blur] from Combobox and TreeView. > (mostly for correctness and NotifyAccessibilityEvent, not critical) > Remove the OnCaretBoundsChanged call from TreeView. > (unnecessary and doesn't set correct candidate window bounds) > Remove unused DesktopRootWindowHost::OnNativeWidget[Focus|Blur]. > Handle NULL TextInputClient* objects in TextInputTestHelper. > > BUG=276720, 271754, 265337 > TEST=Test Win (desktop, Win8 Metro, and Aura builds), CrOS, and Linux Aura's Omnibox, Find Bar, Bookmark Bubble, and Edit Bookmark Dialog's textfield text entry (and text input handling to select combobox and tree view elements) with EN, JA, and other IMEs, and related IME usage (Win+Space to change IME on Win8, Alt+` to change modes, etc.). Also test with --enable-text-services-framework. Note that IME candidate windows may not appear or may appear at incorrect positions for tree views and comboboxes, but this seems like a separate issue. > R=sky@chromium.org > > Review URL: https://chromiumcodereview.appspot.com/23245012 TBR=msw@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220121

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+301 lines, -19 lines) Patch
M trunk/src/chrome/browser/chromeos/input_method/textinput_test_helper.cc View 2 chunks +2 lines, -4 lines 0 comments Download
M trunk/src/ui/views/controls/combobox/combobox.cc View 1 chunk +6 lines, -3 lines 0 comments Download
M trunk/src/ui/views/controls/combobox/native_combobox_views_unittest.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M trunk/src/ui/views/controls/textfield/native_textfield_views.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M trunk/src/ui/views/controls/tree/tree_view.cc View 2 chunks +5 lines, -1 line 0 comments Download
M trunk/src/ui/views/ime/input_method.h View 1 chunk +5 lines, -0 lines 0 comments Download
M trunk/src/ui/views/ime/input_method_base.h View 1 chunk +2 lines, -1 line 0 comments Download
M trunk/src/ui/views/ime/input_method_base.cc View 1 chunk +2 lines, -1 line 0 comments Download
M trunk/src/ui/views/ime/input_method_bridge.h View 4 chunks +36 lines, -1 line 0 comments Download
M trunk/src/ui/views/ime/input_method_bridge.cc View 4 chunks +166 lines, -7 lines 0 comments Download
M trunk/src/ui/views/ime/input_method_bridge_unittest.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M trunk/src/ui/views/ime/mock_input_method.h View 1 chunk +2 lines, -0 lines 0 comments Download
M trunk/src/ui/views/ime/mock_input_method.cc View 2 chunks +5 lines, -0 lines 0 comments Download
M trunk/src/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc View 1 chunk +9 lines, -0 lines 0 comments Download
M trunk/src/ui/views/widget/desktop_aura/desktop_root_window_host.h View 1 chunk +5 lines, -0 lines 0 comments Download
M trunk/src/ui/views/widget/desktop_aura/desktop_root_window_host_win.h View 1 chunk +2 lines, -0 lines 0 comments Download
M trunk/src/ui/views/widget/desktop_aura/desktop_root_window_host_win.cc View 3 chunks +14 lines, -0 lines 0 comments Download
M trunk/src/ui/views/widget/desktop_aura/desktop_root_window_host_x11.h View 1 chunk +2 lines, -0 lines 0 comments Download
M trunk/src/ui/views/widget/desktop_aura/desktop_root_window_host_x11.cc View 2 chunks +10 lines, -0 lines 0 comments Download
M trunk/src/ui/views/widget/native_widget_aura.cc View 1 chunk +14 lines, -1 line 0 comments Download
M trunk/src/ui/views/widget/native_widget_win.cc View 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Shrikant Kelkar
7 years, 3 months ago (2013-08-28 23:52:58 UTC) #1
Shrikant Kelkar
Committed patchset #1 manually as r220121.
7 years, 3 months ago (2013-08-28 23:53:26 UTC) #2
msw
7 years, 3 months ago (2013-08-29 18:02:09 UTC) #3
Message was sent while issue was closed.
LGTM, thanks. I'll try to reland without the regression.

Powered by Google App Engine
This is Rietveld 408576698