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

Issue 419283003: Fix 'repeat' state of a DOM KeyboardEvent on Windows (Closed)

Created:
6 years, 5 months ago by jsbell
Modified:
6 years, 4 months ago
Reviewers:
jamesr, jdduke (slow)
CC:
chromium-reviews, jam, jdduke+watch_chromium.org, darin-cc_chromium.org
Project:
chromium
Visibility:
Public.

Description

Fix 'repeat' state of a DOM KeyboardEvent on Windows The WM_KEYDOWN-to-WebKeyboardEvent mapping was incorrect, probably because MSDN is not helpful. Bits 0-15 of lParam are the number of coalesced events that this message represents (which is 1 unless you're not pumping messages fast enough). Bit 30 of lParam represents the previous key state - i.e. if it was already down; if so, this is a repeat. This manifests in the 'repeat' state of a DOM KeyboardEvent. BUG=263724, 394907 TEST=window.onkeydown=function(e){console.log(e.repeat);}; Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=286090

Patch Set 1 #

Total comments: 1

Patch Set 2 : Use 0xXXXX for mask, add comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -1 line) Patch
M content/browser/renderer_host/input/web_input_event_builders_win.cc View 1 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 12 (0 generated)
jsbell
jdduke@ - please take a look I'm not familiar with this chunk of code; do ...
6 years, 5 months ago (2014-07-25 19:05:38 UTC) #1
jdduke (slow)
jamesr@ might be a better reviewer for this. In any case, a short comment (perhaps ...
6 years, 4 months ago (2014-07-27 21:54:37 UTC) #2
jamesr
https://codereview.chromium.org/419283003/diff/1/content/browser/renderer_host/input/web_input_event_builders_win.cc File content/browser/renderer_host/input/web_input_event_builders_win.cc (right): https://codereview.chromium.org/419283003/diff/1/content/browser/renderer_host/input/web_input_event_builders_win.cc#newcode163 content/browser/renderer_host/input/web_input_event_builders_win.cc:163: if (lparam & (1 << 30)) the rest of ...
6 years, 4 months ago (2014-07-28 19:38:49 UTC) #3
jsbell
On 2014/07/28 19:38:49, jamesr wrote: > https://codereview.chromium.org/419283003/diff/1/content/browser/renderer_host/input/web_input_event_builders_win.cc > File content/browser/renderer_host/input/web_input_event_builders_win.cc > (right): > > https://codereview.chromium.org/419283003/diff/1/content/browser/renderer_host/input/web_input_event_builders_win.cc#newcode163 ...
6 years, 4 months ago (2014-07-28 21:00:50 UTC) #4
jamesr
for treating the actual event. You should be able to run the manual test you ...
6 years, 4 months ago (2014-07-28 21:08:37 UTC) #5
jsbell
On 2014/07/28 21:08:37, jamesr wrote: > for treating the actual event. You should be able ...
6 years, 4 months ago (2014-07-28 21:20:44 UTC) #6
jsbell
The CQ bit was checked by jsbell@chromium.org
6 years, 4 months ago (2014-07-28 21:21:31 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jsbell@chromium.org/419283003/20001
6 years, 4 months ago (2014-07-28 21:22:57 UTC) #8
jsbell
On 2014/07/28 21:22:57, I haz the power (commit-bot) wrote: > CQ is trying da patch. ...
6 years, 4 months ago (2014-07-28 23:48:40 UTC) #9
jdduke (slow)
On 2014/07/28 23:48:40, jsbell wrote: > On 2014/07/28 21:22:57, I haz the power (commit-bot) wrote: ...
6 years, 4 months ago (2014-07-29 00:31:36 UTC) #10
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: chromium_presubmit on tryserver.chromium.linux ...
6 years, 4 months ago (2014-07-29 01:34:58 UTC) #11
commit-bot: I haz the power
6 years, 4 months ago (2014-07-29 04:23:28 UTC) #12
Message was sent while issue was closed.
Change committed as 286090

Powered by Google App Engine
This is Rietveld 408576698