Chromium Code Reviews
Help | Chromium Project | Sign in
(6)

Issue 11280153: Add Search-. as a shortcut for the Insert key when Search is acting as a Function key. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 7 months ago by danakj OOO back july 6
Modified:
2 years, 7 months ago
CC:
chromium-reviews, sadrul, nkostylev+watch_chromium.org, ben+watch_chromium.org, arv, oshima+watch_chromium.org, stevenjb+watch_chromium.org, piman (Very slow to review), backer
Visibility:
Public.

Description

Add Search-. as a shortcut for the Insert key when Search is acting as a Function key. NOTRY=true R=yusukes@chromium.org BUG=162268 Depends on: https://codereview.chromium.org/11421055/ Depends on: https://codereview.chromium.org/11417144/ Depends on: https://codereview.chromium.org/11415124/ Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170847

Patch Set 1 #

Total comments: 7

Patch Set 2 : Rebased and Using . #

Patch Set 3 : Missed a , #

Total comments: 2

Patch Set 4 : 80cols #

Total comments: 2

Patch Set 5 : forlanding #

Messages

Total messages: 23 (0 generated)
danakj OOO back july 6
2 years, 7 months ago (2012-11-24 01:28:05 UTC) #1
danakj OOO back july 6
2 years, 7 months ago (2012-11-24 01:28:10 UTC) #2
Yusuke Sato
Would you mind adding unit tests for this? https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc File chrome/browser/ui/ash/event_rewriter.cc (right): https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc#newcode759 chrome/browser/ui/ash/event_rewriter.cc:759: keysym ...
2 years, 7 months ago (2012-11-26 07:54:44 UTC) #3
danakj OOO back july 6
Oops, forgot a test. Thanks, will add. https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc File chrome/browser/ui/ash/event_rewriter.cc (right): https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc#newcode759 chrome/browser/ui/ash/event_rewriter.cc:759: keysym == ...
2 years, 7 months ago (2012-11-26 17:47:26 UTC) #4
Wez
https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc File chrome/browser/ui/ash/event_rewriter.cc (right): https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc#newcode283 chrome/browser/ui/ash/event_rewriter.cc:283: control_l_xkeycode_ = XKeysymToKeycode(display, XK_Control_L); There are sufficiently many of ...
2 years, 7 months ago (2012-11-26 21:03:19 UTC) #5
Yusuke Sato
https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc File chrome/browser/ui/ash/event_rewriter.cc (right): https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc#newcode759 chrome/browser/ui/ash/event_rewriter.cc:759: keysym == XK_backslash && (xkey->state & Mod4Mask)) { On ...
2 years, 7 months ago (2012-11-26 21:08:28 UTC) #6
danakj OOO back july 6
On 2012/11/26 21:08:28, Yusuke Sato wrote: > https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc > File chrome/browser/ui/ash/event_rewriter.cc (right): > > https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc#newcode759 ...
2 years, 7 months ago (2012-11-26 21:13:05 UTC) #7
Yusuke Sato
On 2012/11/26 21:13:05, danakj wrote: > On 2012/11/26 21:08:28, Yusuke Sato wrote: > > > ...
2 years, 7 months ago (2012-11-30 23:43:07 UTC) #8
Yusuke Sato
https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc File chrome/browser/ui/ash/event_rewriter.cc (right): https://codereview.chromium.org/11280153/diff/1/chrome/browser/ui/ash/event_rewriter.cc#newcode283 chrome/browser/ui/ash/event_rewriter.cc:283: control_l_xkeycode_ = XKeysymToKeycode(display, XK_Control_L); On 2012/11/26 21:03:19, Wez wrote: ...
2 years, 7 months ago (2012-11-30 23:43:55 UTC) #9
danakj OOO back july 6
PTAL
2 years, 7 months ago (2012-12-01 00:30:30 UTC) #10
danakj OOO back july 6
+mazda I think I have the strings in the right places this time, above where ...
2 years, 7 months ago (2012-12-01 00:31:04 UTC) #11
mazda
KeyboardOverlay: lgtm
2 years, 7 months ago (2012-12-01 00:38:44 UTC) #12
Yusuke Sato
lgtm https://codereview.chromium.org/11280153/diff/3010/chrome/browser/ui/ash/event_rewriter_unittest.cc File chrome/browser/ui/ash/event_rewriter_unittest.cc (right): https://codereview.chromium.org/11280153/diff/3010/chrome/browser/ui/ash/event_rewriter_unittest.cc#newcode2393 chrome/browser/ui/ash/event_rewriter_unittest.cc:2393: TEST_F(EventRewriterTest, TestRewriteBackspacePeriodAndArrowKeysWithSearchRemapped) { 80char
2 years, 7 months ago (2012-12-01 15:26:28 UTC) #13
danakj OOO back july 6
Thanks! +derat for event_rewriter OWNERS +sky for chrome OWNERS https://codereview.chromium.org/11280153/diff/3010/chrome/browser/ui/ash/event_rewriter_unittest.cc File chrome/browser/ui/ash/event_rewriter_unittest.cc (right): https://codereview.chromium.org/11280153/diff/3010/chrome/browser/ui/ash/event_rewriter_unittest.cc#newcode2393 chrome/browser/ui/ash/event_rewriter_unittest.cc:2393: ...
2 years, 7 months ago (2012-12-01 18:59:57 UTC) #14
Daniel Erat
lgtm https://codereview.chromium.org/11280153/diff/7002/chrome/browser/ui/ash/event_rewriter.h File chrome/browser/ui/ash/event_rewriter.h (right): https://codereview.chromium.org/11280153/diff/7002/chrome/browser/ui/ash/event_rewriter.h#newcode179 chrome/browser/ui/ash/event_rewriter.h:179: bool RewriteBackspacePeriodAndArrowKeys(ui::KeyEvent* event); this name's getting a bit ...
2 years, 7 months ago (2012-12-01 22:14:11 UTC) #15
sky (OOO until 7-20)
LGTM
2 years, 7 months ago (2012-12-03 15:48:51 UTC) #16
danakj OOO back july 6
https://codereview.chromium.org/11280153/diff/7002/chrome/browser/ui/ash/event_rewriter.h File chrome/browser/ui/ash/event_rewriter.h (right): https://codereview.chromium.org/11280153/diff/7002/chrome/browser/ui/ash/event_rewriter.h#newcode179 chrome/browser/ui/ash/event_rewriter.h:179: bool RewriteBackspacePeriodAndArrowKeys(ui::KeyEvent* event); On 2012/12/01 22:14:12, Daniel Erat wrote: ...
2 years, 7 months ago (2012-12-03 17:46:28 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/11280153/6019
2 years, 7 months ago (2012-12-03 17:49:21 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/11280153/6019
2 years, 7 months ago (2012-12-03 18:45:51 UTC) #19
commit-bot: I haz the power
Commit queue rejected this change because the description was changed between the time the change ...
2 years, 7 months ago (2012-12-04 00:02:39 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/11280153/6019
2 years, 7 months ago (2012-12-04 00:11:53 UTC) #21
commit-bot: I haz the power
Change committed as 170847
2 years, 7 months ago (2012-12-04 00:13:39 UTC) #22
Daniel Erat
2 years, 7 months ago (2012-12-04 08:14:39 UTC) #23
On Mon, Dec 3, 2012 at 9:46 AM,  <danakj@chromium.org> wrote:
>
>
https://codereview.chromium.org/11280153/diff/7002/chrome/browser/ui/ash/even...
> File chrome/browser/ui/ash/event_rewriter.h (right):
>
>
https://codereview.chromium.org/11280153/diff/7002/chrome/browser/ui/ash/even...
> chrome/browser/ui/ash/event_rewriter.h:179: bool
> RewriteBackspacePeriodAndArrowKeys(ui::KeyEvent* event);
> On 2012/12/01 22:14:12, Daniel Erat wrote:
>>
>> this name's getting a bit unwieldy (alas, C doesn't allow commas in
>> identifiers).  would using something like
>
> RewriteFunctionKeysForSearch() and
>>
>> RewriteExtraKeysForSearch() be clearer?
>
>
> I used RewriteExtendedKeys().
>
> The function also rewrites Alt-Backspace and such when not using Search
> key as modifier, so "ForSearch" seemed too restricted.

Thanks, sounds good to me.

> https://codereview.chromium.org/11280153/
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1f9106d