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

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:
1 year, 5 months ago by danakj
Modified:
1 year, 4 months ago
CC:
chromium-reviews_chromium.org, sadrul, nkostylev+watch_chromium.org, ben+watch_chromium.org, arv, 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) Lint Patch
M chrome/app/chromeos_strings.grdp View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments ? errors Download
M chrome/browser/resources/chromeos/keyboard_overlay.js View 1 2 1 chunk +2 lines, -1 line 0 comments ? errors Download
M chrome/browser/ui/ash/event_rewriter.h View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments ? errors Download
M chrome/browser/ui/ash/event_rewriter.cc View 1 2 3 4 3 chunks +7 lines, -2 lines 0 comments 1 errors Download
M chrome/browser/ui/ash/event_rewriter_unittest.cc View 1 2 3 4 7 chunks +21 lines, -4 lines 0 comments ? errors Download
M chrome/browser/ui/webui/chromeos/keyboard_overlay_ui.cc View 1 1 chunk +1 line, -0 lines 0 comments ? errors Download
Commit:

Messages

Total messages: 23
danakj
1 year, 5 months ago #1
danakj
1 year, 5 months ago #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 ...
1 year, 4 months ago #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 == ...
1 year, 4 months ago #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 ...
1 year, 4 months ago #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 ...
1 year, 4 months ago #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 ...
1 year, 4 months ago #7
Yusuke Sato
On 2012/11/26 21:13:05, danakj wrote: > On 2012/11/26 21:08:28, Yusuke Sato wrote: > > > ...
1 year, 4 months ago #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: ...
1 year, 4 months ago #9
danakj
PTAL
1 year, 4 months ago #10
danakj
+mazda I think I have the strings in the right places this time, above where ...
1 year, 4 months ago #11
mazda
KeyboardOverlay: lgtm
1 year, 4 months ago #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
1 year, 4 months ago #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: ...
1 year, 4 months ago #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 ...
1 year, 4 months ago #15
sky
LGTM
1 year, 4 months ago #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: ...
1 year, 4 months ago #17
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/11280153/6019
1 year, 4 months ago #18
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/11280153/6019
1 year, 4 months ago #19
I haz the power (commit-bot)
Commit queue rejected this change because the description was changed between the time the change ...
1 year, 4 months ago #20
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/11280153/6019
1 year, 4 months ago #21
I haz the power (commit-bot)
Change committed as 170847
1 year, 4 months ago #22
Daniel Erat
1 year, 4 months ago #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 1280:2d3e6564b7b6