New issue
Advanced search Search tips

Issue 819860 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug
STS
Team-Accessibility



Sign in to add a comment

[Select-to-Speak] Sticky keys should work with select-to-speak

Project Member Reported by katie@chromium.org, Mar 8 2018

Issue description

Realized we have another interesting StS bug that happens when you enable Sticky Keys (another a11y feature for Chrome OS). That feature makes it possible for you to not need to hold down Search, Ctrl, Alt, Shift to execute commands... for folks with motor challenges. This is actually a good use case to keep in mind, if a user can't hold Search and then also click the touchpad (e.g. if they only have one hand to use). Right now, if you enable Sticky Keys and try to use Select to Speak, it doesn't work... The way it should work is that you press and release Search, then you can use the touchpad to click an item or drag a box, and then once it reads to you, the little UI that shows Search is pressed would then go away and Search would no longer be pressed (that would indicate the end of the action). Does that make sense? Happy to talk more about this in person! 

 

Comment 1 by katie@chromium.org, Mar 9 2018

Labels: -Pri-2 Pri-1

Comment 2 by katie@chromium.org, Mar 12 2018

Status: Started (was: Assigned)
In addition to updating the EventRewriter to happen after Sticky Keys, we should allowing the 's' key to be released before the 'search' key and still read selected text at keystroke.

Comment 3 by katie@chromium.org, Mar 19 2018

Components: UI>Accessibility>SelectToSpeak

Comment 4 by katie@chromium.org, Mar 19 2018

Components: -UI>Accessibility
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51

commit b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51
Author: Katie D <katie@chromium.org>
Date: Fri Mar 30 18:47:36 2018

Adds priorities to EventHandlers, instead of using Prepend/Add.

This will enable us to add a super-high priority, the accessibility
priority, which can go first among the EventHandlers.

It may also help with other bugs related to EventHandler ordering.

Bug:  819860 ,599883
Change-Id: Ie35cc01f8b8d5a15388814e79c7b189d30e7c271
Reviewed-on: https://chromium-review.googlesource.com/967358
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547223}
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/display/touch_calibrator_controller_unittest.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/drag_drop/drag_drop_controller.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/magnifier/docked_magnifier_controller.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/magnifier/magnification_controller_unittest.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/shell.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/shell_unittest.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/system/power/power_button_display_controller.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/system/power/power_button_screenshot_controller.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/utility/screenshot_controller.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/wm/system_gesture_event_filter_unittest.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ash/wm/window_cycle_event_filter_classic.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/chrome/browser/chromeos/login/ui/input_events_blocker.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/chrome/browser/ui/views/javascript_app_modal_event_blocker_x11.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/components/exo/wm_helper.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/components/ui_devtools/views/overlay_agent.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ui/events/BUILD.gn
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ui/events/event_target.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ui/events/event_target.h
[add] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ui/events/event_target_unittest.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ui/events/test/events_test_utils.h
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ui/views/bubble/tray_bubble_view.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ui/views/controls/button/menu_button_unittest.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ui/views/controls/menu/menu_pre_target_handler.cc
[modify] https://crrev.com/b68a552d1f6cb7cccf72f3f9ff8f6c1310713d51/ui/wm/core/focus_controller_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4ea69447605a5534230fbf1751a5fa22464f460c

commit 4ea69447605a5534230fbf1751a5fa22464f460c
Author: Katie D <katie@chromium.org>
Date: Mon Apr 02 23:17:30 2018

Change the STS EventRewriter to an EventHandler at a11y priority.

This makes the STS EventRewriter work with Sticky Keys mode, and
is basically a revert of commit 0c0f37953a732bd7876d209fa81847eab29abb59
along with porting other modifications made since that commit.

Bug:  819860 
Change-Id: I340295ac0ca6e24057fcb9d4a0890d77b3cf0439
Reviewed-on: https://chromium-review.googlesource.com/988849
Commit-Queue: Katie Dektar <katie@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547553}
[modify] https://crrev.com/4ea69447605a5534230fbf1751a5fa22464f460c/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/4ea69447605a5534230fbf1751a5fa22464f460c/chrome/browser/chromeos/accessibility/accessibility_manager.cc
[modify] https://crrev.com/4ea69447605a5534230fbf1751a5fa22464f460c/chrome/browser/chromeos/accessibility/accessibility_manager.h
[rename] https://crrev.com/4ea69447605a5534230fbf1751a5fa22464f460c/chrome/browser/chromeos/accessibility/select_to_speak_event_handler.cc
[rename] https://crrev.com/4ea69447605a5534230fbf1751a5fa22464f460c/chrome/browser/chromeos/accessibility/select_to_speak_event_handler.h
[rename] https://crrev.com/4ea69447605a5534230fbf1751a5fa22464f460c/chrome/browser/chromeos/accessibility/select_to_speak_event_handler_unittest.cc
[modify] https://crrev.com/4ea69447605a5534230fbf1751a5fa22464f460c/chrome/browser/chromeos/chrome_browser_main_chromeos.cc

Comment 7 by katie@chromium.org, Apr 2 2018

Labels: a11y-testers
Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
It's not working for me so I'm reopening and reverting back to Assigned for katie@ to review. 

Google Chrome 67.0.3383.0 (Official Build) dev (64-bit)
Google_Samus.6300.276.0
Chrome OS with flag enabled: #enable-experimental-accessibility-features

# Enable STS
# Enable Sticky keys
# Highlight some text on any page, I used this one: https://en.wikipedia.org/wiki/Tardigrade

# Press and release the Search button, note that the sticky keys menu appears on the upper left corner and the "search" button is highlighted
# Press s while sticky keys menu is still opened
Expected: STS starts reading
Actual: s character is inserted into an edit field, otherwise, nothing

Similarly, I expected that holding down the search key with sticky keys would allow me to then hold down the left mouse cursor and drag a window to invoke the feature that way. However, even when the search key is highlighted, the STS focus ring doesn't happen and it's just a normal selection. 


Labels: -a11y-testers
Labels: M67test

Comment 11 by katie@chromium.org, Apr 10 2018

Status: Fixed (was: Assigned)
I'm still able to get sticky keys to work with STS in 67.0.3394.0 (developer build), using both search+s and search+mouse, even in an edit field.

@leberly, can you please take a video of what you are doing? Or we can chat when I'm back in the office.

Note: You don't need --enable-experimental-accessibility-features any more for STS search+s.

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbe3bf95e4fe2ec5e2563c047e9042ceb5ee8162

commit cbe3bf95e4fe2ec5e2563c047e9042ceb5ee8162
Author: Katie D <katie@chromium.org>
Date: Tue Jun 19 19:14:58 2018

Reorders EventHandlers by priority across all EventTargets.

This is phase 3 of a refactor that allows Select-to-Speak to work
with Sticky Keys, see go/sts-with-sticky-keys for more on the
background for this change.

Not only are EventHandlers ordered by priority across all EventTargets,
but also the later an EventHandler was added to its EventTarget, the
earlier it will appear in the resulting list of EventHandlers. This is
a change for kDefault handlers, but kAccessibility and kSystem already
had this behavior. This is for consistency across all types and requires
a large change to event_dispatcher_unittest.

Also adds an additional debugging function to TestEventHandler
to get that handler's name. This was useful in writing the unittests
for this change and may be useful to others as well.

Bug:  819860 
Change-Id: I4c5002fdc2cd26f0df5e94600183c3cc0cc2ff56
Reviewed-on: https://chromium-review.googlesource.com/996456
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568557}
[modify] https://crrev.com/cbe3bf95e4fe2ec5e2563c047e9042ceb5ee8162/chrome/browser/chromeos/accessibility/select_to_speak_event_handler.cc
[modify] https://crrev.com/cbe3bf95e4fe2ec5e2563c047e9042ceb5ee8162/ui/events/event_target.cc
[modify] https://crrev.com/cbe3bf95e4fe2ec5e2563c047e9042ceb5ee8162/ui/events/event_target_unittest.cc
[modify] https://crrev.com/cbe3bf95e4fe2ec5e2563c047e9042ceb5ee8162/ui/events/test/test_event_handler.h

Sign in to add a comment