MacViews: Space Key doesn't choose/click the currently focused item. |
||||||
Issue descriptionVersion: 52.0.2719.0 OS: Mac What steps will reproduce the problem? (1) Enable chrome://flags/#mac-views-dialogs (2) Open bookmark bubble by clicking on Star icon to the right of address bar. (3) Shift focus to Edit button. (4) Press Space key. What is the expected output? The Edit bookmark dialog should open. What do you see instead? Space Key press is ignored. See https://support.apple.com/en-au/HT204434
,
Jun 1 2016
,
Jun 3 2016
So it turns out, currently we are not getting any Space Key events on MacViews. BridgedContentView::onKeyDown calls interpretKeyEvents. For a space key press, this generates an insertText action message, when inputContext returns nil. This is not routed through doCommandBySelector, so we don't generate a synthetic ui::KeyEvent for this. A simple solution may be to call BridgedContentView::handleKeyEvent in insertText if the string to be inserted has a length of 1. But probably we need to think more about crbug.com/616326 , to decouple key event handling for menus, textfield etc.
,
Jun 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c38e137d2b39cc17c828ddfb783e1842e7dd76f commit 5c38e137d2b39cc17c828ddfb783e1842e7dd76f Author: karandeepb <karandeepb@chromium.org> Date: Tue Jun 21 07:52:39 2016 MacViews: Modify insertText handlers to correctly handle space key and simplify menu event dispatch. Currently, Space key events are not being passed down the view hierarchy, since no ui::KeyEvent is being generated for them. This CL modifies insertText: to generate a synthetic ui::KeyEvent for character events. This allows us to correctly handle the space (and possibly other) keys. Since we no longer call insertText:replacementRange: from insertText:, crbug.com/617110 is also fixed. Also, this CL removes all menu event handling from insertText:replacementRange: since it is only invoked in case of a valid inputContext. Also, this CL adds keyUp: to BridgedContentView, so that key-up events are passed to the Views hierarchy. Also checked that this causes no views_unittests regressions and that enabled interactive_ui_tests in: -menu_controller_interactive_uitest.cc -menu_item_view_interactive_uitest.cc -menu_model_adapter_test.cc -menu_view_drag_and_drop_test.cc still work correctly. BUG= 607429 , 617110 Review-Url: https://codereview.chromium.org/2033433006 Cr-Commit-Position: refs/heads/master@{#400918} [modify] https://crrev.com/5c38e137d2b39cc17c828ddfb783e1842e7dd76f/ui/views/cocoa/bridged_content_view.h [modify] https://crrev.com/5c38e137d2b39cc17c828ddfb783e1842e7dd76f/ui/views/cocoa/bridged_content_view.mm [modify] https://crrev.com/5c38e137d2b39cc17c828ddfb783e1842e7dd76f/ui/views/controls/button/custom_button_unittest.cc [modify] https://crrev.com/5c38e137d2b39cc17c828ddfb783e1842e7dd76f/ui/views/controls/menu/menu_controller.cc [modify] https://crrev.com/5c38e137d2b39cc17c828ddfb783e1842e7dd76f/ui/views/controls/menu/menu_controller.h [modify] https://crrev.com/5c38e137d2b39cc17c828ddfb783e1842e7dd76f/ui/views/controls/menu/menu_key_event_handler.cc
,
Aug 11 2016
,
Dec 12 2016
,
Jan 4 2017
,
Jan 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dfd313b6e9b08551787c6938a04ab1e2814c0f23 commit dfd313b6e9b08551787c6938a04ab1e2814c0f23 Author: karandeepb <karandeepb@chromium.org> Date: Thu Jan 05 10:55:29 2017 MacViews: Handle Space and Return keys correctly for Buttons. Currently on MacViews, the Space key clicks the button on key-release and the Return key clicks the button on key-press if the button has focus. However the Cocoa behavior is that the Space key should click the button on key-press when a button is focused. Also the Return key should perform the default action associated with a dialog, irrespective of the view focused. This CL introduces two new constants to PlatformStyle to account for the special behavior on Mac, namely kClickActionOnSpace and kReturnClicksFocusedControl. CustomButton::OnKeyPressed, CustomButton::OnKeyReleased and CustomButton::SkipDefaultKeyEventProcessing are modified to respect the value of these platform specific constants. Existing unit tests are modified to be in line with the new behavior and one new test is also added. BUG= 607429 , 607430 TEST=On Mac, enable chrome://flags/#secondary-ui-md. Open the Bookmark Bubble. Enable Full Keyboard Access(Ctrl+F7). Focus the Edit button. Verify Space clicks the button on key-press and Return performs the default action(Done) instead of opening the Edit Bookmark Dialog. Review-Url: https://codereview.chromium.org/2607923002 Cr-Commit-Position: refs/heads/master@{#441622} [modify] https://crrev.com/dfd313b6e9b08551787c6938a04ab1e2814c0f23/ui/views/controls/button/custom_button.cc [modify] https://crrev.com/dfd313b6e9b08551787c6938a04ab1e2814c0f23/ui/views/controls/button/custom_button.h [modify] https://crrev.com/dfd313b6e9b08551787c6938a04ab1e2814c0f23/ui/views/controls/button/custom_button_unittest.cc [modify] https://crrev.com/dfd313b6e9b08551787c6938a04ab1e2814c0f23/ui/views/style/platform_style.cc [modify] https://crrev.com/dfd313b6e9b08551787c6938a04ab1e2814c0f23/ui/views/style/platform_style.h [modify] https://crrev.com/dfd313b6e9b08551787c6938a04ab1e2814c0f23/ui/views/style/platform_style_mac.mm [modify] https://crrev.com/dfd313b6e9b08551787c6938a04ab1e2814c0f23/ui/views/window/dialog_delegate_unittest.cc
,
Jan 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b412f671d94f869489023137ba2705785883614f commit b412f671d94f869489023137ba2705785883614f Author: karandeepb <karandeepb@chromium.org> Date: Mon Jan 16 02:34:26 2017 MacViews: Correctly handle character events when there's an active TextInputClient. Currently in insertTextInternal:, we do not generate a synthetic character event for the View hierarchy when there's an active TextInputClient. Since a Space key press generates an insertText: (or insertText:replacementRange:) action message, this causes views::Combobox to not receive the space key press. This is because views::Combobox uses the views::PrefixSelector TextInputClient when focused. As a result, a space key press on a combobox does not currently open the combobox dropdown. To solve, generate key events for all character events in insertTextInternal:. The key event is generated before calling TextInputClient::Insert[Char/Text] which is similar to other platforms. However, the key event is not passed to the View hierarchy in case there's active composition text, since it should be consumed by the IME in that case. Also, a new variable hasUnhandledKeyDownEvent_ is introduced on the BridgedContentView so that keyDownEvent_ remains valid for the duration of keyDown:. This is needed because a keyDown: can infact lead to multiple insertText: action messages. Also, combobox unittests are modified to use an EventGenerator to test key event behavior to provide better coverage. (With an EventGenerator, ComboboxTest.NotifyOnClickWithSpaceKey fails on the master). BUG= 619545 , 607429 TEST=With chrome://flags/#secondary-ui-md on Mac, open the Bookmark bubble. Enable Full Keyboard Access (Ctrl+F7). Focus the Folder combobox. Ensure pressing space opens the combobox dropdown. Review-Url: https://codereview.chromium.org/2624093002 Cr-Commit-Position: refs/heads/master@{#443832} [modify] https://crrev.com/b412f671d94f869489023137ba2705785883614f/ui/views/cocoa/bridged_content_view.h [modify] https://crrev.com/b412f671d94f869489023137ba2705785883614f/ui/views/cocoa/bridged_content_view.mm [modify] https://crrev.com/b412f671d94f869489023137ba2705785883614f/ui/views/controls/combobox/combobox_unittest.cc
,
Jan 27 2017
Status: Combobox, Buttons and Links all respond correctly to Space. Seems to me TreeView and Tabbed pane shouldn't (and they don't currently). Marking as fixed. Also, cc'ing shrike@ and tapted@ in case they know of any different behavior. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by karandeepb@chromium.org
, May 3 2016