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

Issue 11305002: Support TSF related event handling on NativeTextField (Closed)

Created:
8 years, 1 month ago by Seigo Nonaka
Modified:
8 years, 1 month ago
Reviewers:
Peter Kasting, yukawa, sky
CC:
chromium-reviews, tfarina, James Su, penghuang+watch_chromium.org, yusukes+watch_chromium.org
Base URL:
http://git.chromium.org/chromium/src.git@findbar_fix
Visibility:
Public.

Description

Support TSF related event handling on NativeTextField Incremental page search does not work due to TSF unawareness of NativeTextField. I introduced the same approach used in Omnibox (http://crrev.com/163233). BUG=157702 TEST=Manually done on Windows 8 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164667

Patch Set 1 : #

Total comments: 4

Patch Set 2 : Reorder functions #

Total comments: 8

Patch Set 3 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+177 lines, -56 lines) Patch
M chrome/browser/ui/views/omnibox/omnibox_view_win.h View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/omnibox/omnibox_view_win.cc View 1 2 2 chunks +11 lines, -11 lines 0 comments Download
M ui/base/ime/win/tsf_event_router.h View 1 2 3 chunks +18 lines, -8 lines 0 comments Download
M ui/base/ime/win/tsf_event_router.cc View 1 7 chunks +56 lines, -18 lines 0 comments Download
M ui/views/controls/textfield/native_textfield_win.h View 1 2 7 chunks +16 lines, -1 line 0 comments Download
M ui/views/controls/textfield/native_textfield_win.cc View 1 2 8 chunks +74 lines, -16 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
Seigo Nonaka
8 years, 1 month ago (2012-10-26 08:06:16 UTC) #1
Yohei Yukawa
lgtm
8 years, 1 month ago (2012-10-27 09:11:14 UTC) #2
Seigo Nonaka
Yukawa: Thank you for your review: Adding sky@ as the owner of ui/views Adding pkasting@ ...
8 years, 1 month ago (2012-10-27 15:53:19 UTC) #3
Peter Kasting
http://codereview.chromium.org/11305002/diff/3002/chrome/browser/ui/views/omnibox/omnibox_view_win.cc File chrome/browser/ui/views/omnibox/omnibox_view_win.cc (right): http://codereview.chromium.org/11305002/diff/3002/chrome/browser/ui/views/omnibox/omnibox_view_win.cc#newcode931 chrome/browser/ui/views/omnibox/omnibox_view_win.cc:931: void OmniboxViewWin::OnTsfStartComposition() {} Does this message give us what ...
8 years, 1 month ago (2012-10-27 19:00:46 UTC) #4
Seigo Nonaka
http://codereview.chromium.org/11305002/diff/3002/chrome/browser/ui/views/omnibox/omnibox_view_win.cc File chrome/browser/ui/views/omnibox/omnibox_view_win.cc (right): http://codereview.chromium.org/11305002/diff/3002/chrome/browser/ui/views/omnibox/omnibox_view_win.cc#newcode931 chrome/browser/ui/views/omnibox/omnibox_view_win.cc:931: void OmniboxViewWin::OnTsfStartComposition() {} Unfortunately no, because OnTsfstartComosition is called ...
8 years, 1 month ago (2012-10-27 22:20:18 UTC) #5
Peter Kasting
http://codereview.chromium.org/11305002/diff/21001/chrome/browser/ui/views/omnibox/omnibox_view_win.h File chrome/browser/ui/views/omnibox/omnibox_view_win.h (right): http://codereview.chromium.org/11305002/diff/21001/chrome/browser/ui/views/omnibox/omnibox_view_win.h#newcode332 chrome/browser/ui/views/omnibox/omnibox_view_win.h:332: // TsfEventRouter::Observer Nit: Add trailing colon http://codereview.chromium.org/11305002/diff/21001/chrome/browser/ui/views/omnibox/omnibox_view_win.h#newcode333 chrome/browser/ui/views/omnibox/omnibox_view_win.h:333: virtual ...
8 years, 1 month ago (2012-10-27 22:29:50 UTC) #6
Seigo Nonaka
http://codereview.chromium.org/11305002/diff/21001/chrome/browser/ui/views/omnibox/omnibox_view_win.h File chrome/browser/ui/views/omnibox/omnibox_view_win.h (right): http://codereview.chromium.org/11305002/diff/21001/chrome/browser/ui/views/omnibox/omnibox_view_win.h#newcode332 chrome/browser/ui/views/omnibox/omnibox_view_win.h:332: // TsfEventRouter::Observer On 2012/10/27 22:29:50, Peter Kasting wrote: > ...
8 years, 1 month ago (2012-10-28 01:50:44 UTC) #7
Peter Kasting
LGTM
8 years, 1 month ago (2012-10-28 21:03:01 UTC) #8
Seigo Nonaka
pkating: Thank you for your review! sky: PTAL? Thanks.
8 years, 1 month ago (2012-10-29 02:03:53 UTC) #9
sky
LGTM
8 years, 1 month ago (2012-10-29 14:48:10 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/nona@chromium.org/11305002/27002
8 years, 1 month ago (2012-10-29 14:49:28 UTC) #11
commit-bot: I haz the power
Change committed as 164667
8 years, 1 month ago (2012-10-29 17:22:16 UTC) #12
Finnur
8 years, 1 month ago (2012-10-31 15:57:09 UTC) #13
I suspect this CL of breaking focus for Find in page -- at least it certainly
falls within the regression range (164656-164681) and looks very related.

See http://crbug.com/158562

Powered by Google App Engine
This is Rietveld 408576698