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

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

Created:
8 years ago by danakj
Modified:
8 years ago
CC:
chromium-reviews, sadrul, nkostylev+watch_chromium.org, ben+watch_chromium.org, arv (Not doing code reviews), oshima+watch_chromium.org, stevenjb+watch_chromium.org, piman, 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 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -9 lines) Patch
M chrome/app/chromeos_strings.grdp View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/resources/chromeos/keyboard_overlay.js View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/ash/event_rewriter.h View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/event_rewriter.cc View 1 2 3 4 3 chunks +7 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/event_rewriter_unittest.cc View 1 2 3 4 7 chunks +21 lines, -4 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/keyboard_overlay_ui.cc View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 23 (0 generated)
danakj
8 years ago (2012-11-24 01:28:05 UTC) #1
danakj
8 years 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 ...
8 years ago (2012-11-26 07:54:44 UTC) #3
danakj
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 == ...
8 years 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 ...
8 years 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 ...
8 years ago (2012-11-26 21:08:28 UTC) #6
danakj
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 ...
8 years 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: > > > ...
8 years 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: ...
8 years ago (2012-11-30 23:43:55 UTC) #9
danakj
PTAL
8 years ago (2012-12-01 00:30:30 UTC) #10
danakj
+mazda I think I have the strings in the right places this time, above where ...
8 years ago (2012-12-01 00:31:04 UTC) #11
mazda
KeyboardOverlay: lgtm
8 years 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
8 years ago (2012-12-01 15:26:28 UTC) #13
danakj
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: ...
8 years 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 ...
8 years ago (2012-12-01 22:14:11 UTC) #15
sky
LGTM
8 years ago (2012-12-03 15:48:51 UTC) #16
danakj
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: ...
8 years 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
8 years 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
8 years 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 ...
8 years 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
8 years ago (2012-12-04 00:11:53 UTC) #21
commit-bot: I haz the power
Change committed as 170847
8 years ago (2012-12-04 00:13:39 UTC) #22
Daniel Erat
8 years 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/

Powered by Google App Engine
This is Rietveld 408576698