Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1458203003:
MacKeyboard: Don't generate keypress for non-printable char (Closed)
|
Created:
5 years, 1 month ago by chongz Modified:
5 years ago CC:
chromium-reviews, darin-cc_chromium.org, jam, nona+watch_chromium.org, shuchen+watch_chromium.org, James Su, vmpstr+watch_chromium.org, yusukes+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMacKeyboard: Don't generate keypress for non-printable char
---Before CL:
Press Delete:
Linux: keypress (charCode=127)
Windows, Mac: no keypress
Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac)
Mac: keypress (charCode=0xF728)
Windows, Linux: no keypress
Press Ctrl+F12:
Mac: keypress (charCode=0xF70F)
Window, Linux: no keypress
---After CL:
no keypress for everyone
BUG=459089
Committed: https://crrev.com/0cb4bd70989a22ea2e31590f291c0b2cc7d27433
Cr-Commit-Position: refs/heads/master@{#366205}
Committed: https://crrev.com/c8bec73a8b23de3fe11b4cbc2fb9fa0c0020e5c0
Cr-Commit-Position: refs/heads/master@{#366461}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Moved StringIsPrintable and Updated Tests #Patch Set 3 : Filter more private use characters #
Total comments: 9
Patch Set 4 : erikchen and dtapuska's review #
Total comments: 1
Patch Set 5 : Fixed if-else style, added layout tests #Patch Set 6 : Added a comparison test case #Patch Set 7 : Removed incorrect layout tests #Patch Set 8 : Suppress NSDeleteFunctionKey on Mac-side #Patch Set 9 : Rebase and use new ShutdownAndDestroyWidget() method #
Messages
Total messages: 48 (17 generated)
Description was changed from ========== MacKeyboard: Don't generate keypress for non-printable char This CL added a filter in [RenderWidgetHostViewMac keyevent:], so it won't generate keypress or Char event from a NSEvent with only non-printable characters in it. e.g. On mac fn-ctrl-delete won't insert a private use character anymore BUG=459089 ========== to ========== MacKeyboard: Don't generate keypress for non-printable char This CL added a filter in [RenderWidgetHostViewMac keyevent:], so it won't generate keypress or Char event from a NSEvent with only non-printable characters in it. e.g. On mac fn-ctrl-delete won't insert a private use character anymore BUG=459089 ==========
chongz@chromium.org changed reviewers: + dtapuska@chromium.org
https://codereview.chromium.org/1458203003/diff/1/base/mac/mac_util.h File base/mac/mac_util.h (right): https://codereview.chromium.org/1458203003/diff/1/base/mac/mac_util.h#newcode125 base/mac/mac_util.h:125: BASE_EXPORT bool StringIsPrintable(NSString* s); This is too generic in the base.. Since you are only testing it inside the scope of the render_host_view_mac I'd move it to an anonymous function https://codereview.chromium.org/1458203003/diff/1/content/browser/renderer_ho... File content/browser/renderer_host/render_widget_host_view_mac_unittest.mm (right): https://codereview.chromium.org/1458203003/diff/1/content/browser/renderer_ho... content/browser/renderer_host/render_widget_host_view_mac_unittest.mm:353: EXPECT_EQ(1U, process_host->sink().message_count()); It's unclear to me how this message_count increases? Do you want to assert it is 1 before the key event is sent; and then it doesn't in fact change?
On 2015/11/23 14:42:08, dtapuska wrote: > https://codereview.chromium.org/1458203003/diff/1/base/mac/mac_util.h > File base/mac/mac_util.h (right): > > https://codereview.chromium.org/1458203003/diff/1/base/mac/mac_util.h#newcode125 > base/mac/mac_util.h:125: BASE_EXPORT bool StringIsPrintable(NSString* s); > This is too generic in the base.. > > Since you are only testing it inside the scope of the render_host_view_mac I'd > move it to an anonymous function > > https://codereview.chromium.org/1458203003/diff/1/content/browser/renderer_ho... > File content/browser/renderer_host/render_widget_host_view_mac_unittest.mm > (right): > > https://codereview.chromium.org/1458203003/diff/1/content/browser/renderer_ho... > content/browser/renderer_host/render_widget_host_view_mac_unittest.mm:353: > EXPECT_EQ(1U, process_host->sink().message_count()); > It's unclear to me how this message_count increases? > > Do you want to assert it is 1 before the key event is sent; and then it doesn't > in fact change? I have made a copy of 'GetInputMessageTypes' in 'render_widget_host_view_mac_unittest' from 'render_widget_host_unittest' to avoid messing with includes.. In the test a non-printable key will only generate 'RawKeyDown', while a printable key will generate 'RawKeyDown' and 'Char'.
On 2015/11/24 20:07:24, chongz wrote: lgtm
dtapuska@chromium.org changed reviewers: + ccameron@chromium.org
ccameron@chromium.org changed reviewers: + erikchen@chromium.org
->erickchen, since he wrote EventIsReservedBySystem, which looks to be the mechanism that we should be using here.
On 2015/11/30 20:37:57, ccameron wrote: > ->erickchen, since he wrote EventIsReservedBySystem, which looks to be the > mechanism that we should be using here. It's not clear to me that this is the right approach. I imagine that we should handle the forward delete input event the same way that we handle the backwards delete input event? And if we choose not to handle it, we should choose to do that in the same location? It looks like Chrome tries to do something with this character, and then passes the result to Blink. Perhaps Blink is incorrectly handling the event? https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re... https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... Note that: ./System/Library/Frameworks/AppKit.framework/Headers/NSEvent.h: NSDeleteFunctionKey = 0xF728,
Description was changed from ========== MacKeyboard: Don't generate keypress for non-printable char This CL added a filter in [RenderWidgetHostViewMac keyevent:], so it won't generate keypress or Char event from a NSEvent with only non-printable characters in it. e.g. On mac fn-ctrl-delete won't insert a private use character anymore BUG=459089 ========== to ========== MacKeyboard: Don't generate keypress for non-printable char 1. Map NSDeleteFunctionKey to 0x7F (mac only), and filter 0x7F in blink. 2. Map other Apple private use characters to empty text, and don't generate keypress for them. e.g. On mac fn-ctrl-backspace or ctrl-F12 won't insert a private use character anymore BUG=459089 ==========
Description was changed from ========== MacKeyboard: Don't generate keypress for non-printable char 1. Map NSDeleteFunctionKey to 0x7F (mac only), and filter 0x7F in blink. 2. Map other Apple private use characters to empty text, and don't generate keypress for them. e.g. On mac fn-ctrl-backspace or ctrl-F12 won't insert a private use character anymore BUG=459089 ========== to ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 ==========
On 2015/11/30 21:51:53, erikchen wrote: > On 2015/11/30 20:37:57, ccameron wrote: > > ->erickchen, since he wrote EventIsReservedBySystem, which looks to be the > > mechanism that we should be using here. > > It's not clear to me that this is the right approach. I imagine that we should > handle the forward delete input event the same way that we handle the backwards > delete input event? And if we choose not to handle it, we should choose to do > that in the same location? > > It looks like Chrome tries to do something with this character, and then passes > the result to Blink. Perhaps Blink is incorrectly handling the event? > https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re... > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > Note that: > ./System/Library/Frameworks/AppKit.framework/Headers/NSEvent.h: > NSDeleteFunctionKey = 0xF728, After a long discussion, another solution would be map Apple private use key first and then do the filter by char code, hope this is the right approach. Thanks!
https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... File content/browser/renderer_host/input/web_input_event_builders_mac.mm (right): https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... content/browser/renderer_host/input/web_input_event_builders_mac.mm:635: if ([text_str length] == 1) { Can we do this in TextFromEvent and UnmodifiedTextFromEvent? And put all the filtering code into a single function? https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... File content/browser/renderer_host/render_widget_host_view_mac.mm (right): https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... content/browser/renderer_host/render_widget_host_view_mac.mm:2303: event.text[0] != '\0' && Can we use [[event text] length] > 0 ?
https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... File content/browser/renderer_host/input/web_input_event_builders_mac.mm (right): https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... content/browser/renderer_host/input/web_input_event_builders_mac.mm:640: } else if (c >= 0xF700 && c <= 0xF747) { Where are you getting 0xF747 from? https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... seems to imply f7ff or f8ff might be a better choice. Either way, can you add a comment with a link to an appropriate reference? https://codereview.chromium.org/1458203003/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right): https://codereview.chromium.org/1458203003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/web/WebInputEventConversion.cpp:326: // Please refer to bug http://b/issue?id=961192, which talks about Webkit This comment looks out of date. Can you update it, or just remove it?
https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... File content/browser/renderer_host/input/web_input_event_builders_mac.mm (right): https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... content/browser/renderer_host/input/web_input_event_builders_mac.mm:640: } else if (c >= 0xF700 && c <= 0xF747) { On 2015/12/10 21:48:09, erikchen wrote: > Where are you getting 0xF747 from? > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > > seems to imply f7ff or f8ff might be a better choice. Either way, can you add a > comment with a link to an appropriate reference? F7FF I'd prefer; would match this: http://www.opensource.apple.com/source/WebCore/WebCore-7601.1.55/platform/mac...
PTAL And I'm not sure is my change related to "mojo:mash_wm_apptests"? (it failed) Thanks! https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... File content/browser/renderer_host/input/web_input_event_builders_mac.mm (right): https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... content/browser/renderer_host/input/web_input_event_builders_mac.mm:635: if ([text_str length] == 1) { On 2015/12/10 20:50:12, dtapuska wrote: > Can we do this in TextFromEvent and UnmodifiedTextFromEvent? > > And put all the filtering code into a single function? Moved text_str filter into a single function, but should I create another funtion for result.windowsKeyCode as well? https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... content/browser/renderer_host/input/web_input_event_builders_mac.mm:640: } else if (c >= 0xF700 && c <= 0xF747) { On 2015/12/10 21:53:01, dtapuska wrote: > On 2015/12/10 21:48:09, erikchen wrote: > > Where are you getting 0xF747 from? > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > > > > > seems to imply f7ff or f8ff might be a better choice. Either way, can you add > a > > comment with a link to an appropriate reference? > > > F7FF I'd prefer; would match this: > > http://www.opensource.apple.com/source/WebCore/WebCore-7601.1.55/platform/mac... Done. https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... File content/browser/renderer_host/render_widget_host_view_mac.mm (right): https://codereview.chromium.org/1458203003/diff/40001/content/browser/rendere... content/browser/renderer_host/render_widget_host_view_mac.mm:2303: event.text[0] != '\0' && On 2015/12/10 20:50:12, dtapuska wrote: > Can we use [[event text] length] > 0 ? Probably not because event.text is a WebUChar array, and also 'strlen' is unsafe when the string does not end with '\0' https://codereview.chromium.org/1458203003/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right): https://codereview.chromium.org/1458203003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/web/WebInputEventConversion.cpp:326: // Please refer to bug http://b/issue?id=961192, which talks about Webkit On 2015/12/10 21:48:09, erikchen wrote: > This comment looks out of date. Can you update it, or just remove it? Done.
https://codereview.chromium.org/1458203003/diff/60001/content/browser/rendere... File content/browser/renderer_host/input/web_input_event_builders_mac.mm (right): https://codereview.chromium.org/1458203003/diff/60001/content/browser/rendere... content/browser/renderer_host/input/web_input_event_builders_mac.mm:126: // Backspace should be 8 I'd assume you have braces here because the code for the else continues into the next line because of the comment. if (c == 0x7f) result = @"\x8"; // Backspace should be 8 else if (c .... result = ... else if result = ... Is the typical style where comments are placed on the same line of the executable path of the conditional; but the last else condition has 3 lines of comments. So you might want to put braces on all conditionals and call it a day. Ultimately the goal is quick readability so you don't need to parse the code.
On 2015/12/11 16:20:30, dtapuska wrote: > https://codereview.chromium.org/1458203003/diff/60001/content/browser/rendere... > File content/browser/renderer_host/input/web_input_event_builders_mac.mm > (right): > > https://codereview.chromium.org/1458203003/diff/60001/content/browser/rendere... > content/browser/renderer_host/input/web_input_event_builders_mac.mm:126: // > Backspace should be 8 > I'd assume you have braces here because the code for the else continues into the > next line because of the comment. > > if (c == 0x7f) > result = @"\x8"; // Backspace should be 8 > else if (c .... > result = ... > else if > result = ... > > Is the typical style where comments are placed on the same line of the > executable path of the conditional; but the last else condition has 3 lines of > comments. So you might want to put braces on all conditionals and call it a day. > > Ultimately the goal is quick readability so you don't need to parse the code. PTAL, fixed if-else style and added layout tests for special characters. Chrome behavior before and after CL are: ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everything above
On 2015/12/14 02:31:22, chongz wrote: > On 2015/12/11 16:20:30, dtapuska wrote: > > > https://codereview.chromium.org/1458203003/diff/60001/content/browser/rendere... > > File content/browser/renderer_host/input/web_input_event_builders_mac.mm > > (right): > > > > > https://codereview.chromium.org/1458203003/diff/60001/content/browser/rendere... > > content/browser/renderer_host/input/web_input_event_builders_mac.mm:126: // > > Backspace should be 8 > > I'd assume you have braces here because the code for the else continues into > the > > next line because of the comment. > > > > if (c == 0x7f) > > result = @"\x8"; // Backspace should be 8 > > else if (c .... > > result = ... > > else if > > result = ... > > > > Is the typical style where comments are placed on the same line of the > > executable path of the conditional; but the last else condition has 3 lines of > > comments. So you might want to put braces on all conditionals and call it a > day. > > > > Ultimately the goal is quick readability so you don't need to parse the code. > > PTAL, fixed if-else style and added layout tests for special characters. > > Chrome behavior before and after CL are: > ---Before CL: > Press Delete: > Linux: keypress (charCode=127) > Windows, Mac: no keypress > > Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) > Mac: keypress (charCode=0xF728) > Windows, Linux: no keypress > > Press Ctrl+F12: > Mac: keypress (charCode=0xF70F) > Window, Linux: no keypress > > ---After CL: > no keypress for everything above lgtm
This looks approximately correct to me, though I am not an expert on event handling. lgtm.
chongz@chromium.org changed reviewers: + aelias@chromium.org
Hi aelias, can you have a look at my CL please? I want to make sure I'm doing it in the right way. Thanks!
Could you add test cases (can be just more clauses in your existing layout test) for other nonprintable characters including arrow keys, ctrl, alt, shift, caps lock? I assume the "text().length() == 0" line is for those and it would be good to have test coverage.
On 2015/12/15 01:06:27, aelias wrote: > Could you add test cases (can be just more clauses in your existing layout test) > for other nonprintable characters including arrow keys, ctrl, alt, shift, caps Actually removed the layout test as it won't hit my code, see: https://code.google.com/p/chromium/codesearch#chromium/src/components/test_ru... where 'eventSender.keyDown' will send 'keypress' if it's not a special key, so the test will always pass. And simulating events using other JavaScript methods won't hit my code either. > lock? I assume the "text().length() == 0" line is for those and it would be > good to have test coverage. For me "text().length() == 0" is just a pre-check before I access 'text().characterStartingAt(0)'. Only Backspace, Escape and Delete will go into here and other special keys won't cause Char events. What I've changed is just add a filter for Delete, and I couldn't find how we do tests for other keys before... But I'd imagine the full test should be from NativeEvents across IPC to JS events? Or should I add a test for 'PlatformKeyboardEventBuilder::isCharacterKey()'? Thanks!
Hmm, looking more closely at this, it seems like it's primarily browser-side code's responsibility not to send events for these. You even observed only Linux sends delete KeyPress, right? In general, redundant "defensive" code like this is discouraged, it's better for only one subsystem to be responsible for it. (The problem seems preexisting as I don't think it should exist for escape and backspace either.) I've been thinking it would be a better approach if the browser only ever sent KeyDown events, and all KeyPress events were synthesized at the last minute in Blink. It would avoid platform inconsistencies like this. For this patch, I'd suggest just removing the Blink change as it doesn't seem like the primary bug you're intending to fix?
On 2015/12/16 07:42:05, aelias wrote: > Hmm, looking more closely at this, it seems like it's primarily browser-side > code's responsibility not to send events for these. You even observed only > Linux sends delete KeyPress, right? In general, redundant "defensive" code like > this is discouraged, it's better for only one subsystem to be responsible for > it. (The problem seems preexisting as I don't think it should exist for escape > and backspace either.) > > I've been thinking it would be a better approach if the browser only ever sent > KeyDown events, and all KeyPress events were synthesized at the last minute in I see, it might be better to open another issue for this. > Blink. It would avoid platform inconsistencies like this. For this patch, I'd > suggest just removing the Blink change as it doesn't seem like the primary bug > you're intending to fix? For my patch since ctrl+delete on Mac will generate a private use character (single delete won't), my solution is 1. Convert private use character to 0x7F in RenderWidgetHostViewMac and pass to Blink as a keypress (to match Linux) 2. Block 0x7F in Blink for both Linux and Mac Another solution would be don't forward keypress for ctrl+delete on Mac (probably what windows does), then I can remove the Blink change. Would this be better? Thanks!
On 2015/12/16 at 16:23:11, chongz wrote: > For my patch since ctrl+delete on Mac will generate a private use character (single delete won't), my solution is > 1. Convert private use character to 0x7F in RenderWidgetHostViewMac and pass to Blink as a keypress (to match Linux) > 2. Block 0x7F in Blink for both Linux and Mac > > Another solution would be don't forward keypress for ctrl+delete on Mac (probably what windows does), then I can remove the Blink change. > Would this be better? > > Thanks! Yes I think it would be better since it would avoid unnecessary code. My understanding is that if you just delete both the "if (c == NSDeleteFunctionKey)" block on the browser side and the Blink-side change, the behavior would still be as you intend on Mac (since NSDeleteFunctionKey == 0xF728 and would go into the private code suppression block). Please give it a try.
Hi aelias, I've removed the blink code and suppress delete key on Mac-side, tested and works nice. PTAL Thanks!
On 2015/12/18 at 19:09:25, chongz wrote: > Hi aelias, > > I've removed the blink code and suppress delete key on Mac-side, tested and works nice. > PTAL > > Thanks! lgtm, thanks!
The CQ bit was checked by chongz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dtapuska@chromium.org, erikchen@chromium.org Link to the patchset: https://codereview.chromium.org/1458203003/#ps140001 (title: "Suppress NSDeleteFunctionKey on Mac-side")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1458203003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1458203003/140001
Message was sent while issue was closed.
Description was changed from ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 ========== to ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 ========== to ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 Committed: https://crrev.com/0cb4bd70989a22ea2e31590f291c0b2cc7d27433 Cr-Commit-Position: refs/heads/master@{#366205} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/0cb4bd70989a22ea2e31590f291c0b2cc7d27433 Cr-Commit-Position: refs/heads/master@{#366205}
Message was sent while issue was closed.
danakj@chromium.org changed reviewers: + danakj@chromium.org
Message was sent while issue was closed.
I think this broke compile. Maybe needs a rebase. https://build.chromium.org/p/chromium.mac/builders/Mac%20Builder/builds/33673
Message was sent while issue was closed.
A revert of this CL (patchset #8 id:140001) has been created in https://codereview.chromium.org/1540013004/ by danakj@chromium.org. The reason for reverting is: Needs a rebase. Breaks compile..
Message was sent while issue was closed.
Description was changed from ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 Committed: https://crrev.com/0cb4bd70989a22ea2e31590f291c0b2cc7d27433 Cr-Commit-Position: refs/heads/master@{#366205} ========== to ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 Committed: https://crrev.com/0cb4bd70989a22ea2e31590f291c0b2cc7d27433 Cr-Commit-Position: refs/heads/master@{#366205} ==========
The CQ bit was checked by chongz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org, dtapuska@chromium.org, erikchen@chromium.org Link to the patchset: https://codereview.chromium.org/1458203003/#ps160001 (title: "Rebase and use new ShutdownAndDestroyWidget() method")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1458203003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1458203003/160001
Message was sent while issue was closed.
Description was changed from ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 Committed: https://crrev.com/0cb4bd70989a22ea2e31590f291c0b2cc7d27433 Cr-Commit-Position: refs/heads/master@{#366205} ========== to ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 Committed: https://crrev.com/0cb4bd70989a22ea2e31590f291c0b2cc7d27433 Cr-Commit-Position: refs/heads/master@{#366205} ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 Committed: https://crrev.com/0cb4bd70989a22ea2e31590f291c0b2cc7d27433 Cr-Commit-Position: refs/heads/master@{#366205} ========== to ========== MacKeyboard: Don't generate keypress for non-printable char ---Before CL: Press Delete: Linux: keypress (charCode=127) Windows, Mac: no keypress Press Ctrl+Delete: (or Fn+Ctrl+Delete on Mac) Mac: keypress (charCode=0xF728) Windows, Linux: no keypress Press Ctrl+F12: Mac: keypress (charCode=0xF70F) Window, Linux: no keypress ---After CL: no keypress for everyone BUG=459089 Committed: https://crrev.com/0cb4bd70989a22ea2e31590f291c0b2cc7d27433 Cr-Commit-Position: refs/heads/master@{#366205} Committed: https://crrev.com/c8bec73a8b23de3fe11b4cbc2fb9fa0c0020e5c0 Cr-Commit-Position: refs/heads/master@{#366461} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/c8bec73a8b23de3fe11b4cbc2fb9fa0c0020e5c0 Cr-Commit-Position: refs/heads/master@{#366461} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
