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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 11580005: Don't fire Alt-Search binding when Alt-Search is being used for accessing a modifer key. (Closed)

Created:
8 years ago by danakj
Modified:
7 years, 11 months ago
CC:
chromium-reviews, mazda+watch_chromium.org, sadrul, yusukes+watch_chromium.org, derat+watch_chromium.org, ben+watch_chromium.org
Visibility:
Public.

Description

Don't fire Alt-Search binding when Alt-Search is being used for accessing a modifer key. BUG=165327 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173616

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : rebase #

Total comments: 2

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -3 lines) Patch
M ash/accelerators/accelerator_controller.cc View 1 2 3 1 chunk +9 lines, -0 lines 0 comments Download
M ash/accelerators/accelerator_controller_unittest.cc View 1 2 1 chunk +6 lines, -2 lines 0 comments Download
M ash/accelerators/accelerator_table.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 8 (0 generated)
danakj
8 years ago (2012-12-17 20:20:17 UTC) #1
danakj
yusukes@ is OOO today, could you give this a review please mazda@?
8 years ago (2012-12-17 20:37:29 UTC) #2
Daniel Erat
LGTM if you need approval (will let other reviews comment on correctness)
8 years ago (2012-12-17 20:43:38 UTC) #3
danakj
I filed crbug.com/166495 for the Alt-released-first case.
8 years ago (2012-12-17 21:37:55 UTC) #4
mazda
lgtm with a nit https://codereview.chromium.org/11580005/diff/7001/ash/accelerators/accelerator_controller.cc File ash/accelerators/accelerator_controller.cc (right): https://codereview.chromium.org/11580005/diff/7001/ash/accelerators/accelerator_controller.cc#newcode589 ash/accelerators/accelerator_controller.cc:589: // Search key. nit: Please ...
8 years ago (2012-12-17 21:41:54 UTC) #5
danakj
Thanks! https://codereview.chromium.org/11580005/diff/7001/ash/accelerators/accelerator_controller.cc File ash/accelerators/accelerator_controller.cc (right): https://codereview.chromium.org/11580005/diff/7001/ash/accelerators/accelerator_controller.cc#newcode589 ash/accelerators/accelerator_controller.cc:589: // Search key. On 2012/12/17 21:41:54, mazda wrote: ...
8 years ago (2012-12-17 21:48:02 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/11580005/6002
8 years ago (2012-12-17 21:58:52 UTC) #7
commit-bot: I haz the power
8 years ago (2012-12-18 01:14:26 UTC) #8

Powered by Google App Engine
This is Rietveld 408576698