New issue
Advanced search Search tips

Issue 867110 link

Starred by 1 user
Status: Fixed
Owner:
jamescook@chromium.org
Closed: Aug 2
Cc:
mustash-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug
Team-Accessibility

Blocking:
issue 841020



Sign in to add a comment

Make caret highlight a11y work in shortcut viewer mojo app

Project Member Reported by jamescook@chromium.org, Jul 24 
Right now we fall back to the old non-app shortcut viewer if text caret highlighting is enabled.

Code is in ash/accessibility in AccessibilityHighlightController.

It might be an InputMethodObserver problem.

Fixing this will probably fix focus highlights as well.

Comment 1 by jamescook@chromium.org, Jul 24

Blocking: 841020
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 26 

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

commit 2094f0159a03dc63bb8a34ddcf68d46d1a7c5355
Author: James Cook <jamescook@chromium.org>
Date: Thu Jul 26 23:06:32 2018

chromeos: Support a11y caret highlight in shortcut viewer app and mash

The existing code updates the text input caret highlight by observing
input method state. This works in classic ash because the input method
is shared globally across ash and browser code. This doesn't work with
remote apps because ash can't see the input method in the remote
process. The caret doesn't work in mash because the ash process can't
see the browser's input method.

Add support for a11y to observe the input method objects created by the
browser's input method driver / bridge. Explicitly forward the
text caret bounds to ash, which then updates the highlight.

Bug:  867110 
Test: added to ash_unittests and browser_tests
Change-Id: I27058e091ad5a597de4ab644654df8d535e95b72
Reviewed-on: https://chromium-review.googlesource.com/1151897
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578473}
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/ash/accessibility/accessibility_controller.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/ash/accessibility/accessibility_controller.h
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/ash/accessibility/accessibility_highlight_controller.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/ash/accessibility/accessibility_highlight_controller.h
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/ash/accessibility/accessibility_highlight_controller_unittest.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/ash/display/window_tree_host_manager.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/ash/public/interfaces/accessibility_controller.mojom
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/ash/shell.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/ash/shell.h
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/chromeos/BUILD.gn
[add] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/chromeos/accessibility/accessibility_input_method_observer.cc
[add] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/chromeos/accessibility/accessibility_input_method_observer.h
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/chromeos/accessibility/accessibility_manager.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/chromeos/accessibility/accessibility_manager.h
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/ui/ash/accessibility/accessibility_controller_client_unittest.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_util.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/ui/views/ime_driver/ime_driver_mus.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/ui/views/ime_driver/ime_driver_mus.h
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc
[modify] https://crrev.com/2094f0159a03dc63bb8a34ddcf68d46d1a7c5355/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.h
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 27 

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

commit ee3b12babcf98ab2020c827de2fb1e313e0102e5
Author: Yoichi Osato <yoichio@chromium.org>
Date: Fri Jul 27 07:16:42 2018

Revert "chromeos: Support a11y caret highlight in shortcut viewer app and mash"

This reverts commit 2094f0159a03dc63bb8a34ddcf68d46d1a7c5355.

Reason for revert: viz_browser_tests failing on chromium.memory/Linux Chromium OS ASan LSan Tests
https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/28406

Original change's description:
> chromeos: Support a11y caret highlight in shortcut viewer app and mash
> 
> The existing code updates the text input caret highlight by observing
> input method state. This works in classic ash because the input method
> is shared globally across ash and browser code. This doesn't work with
> remote apps because ash can't see the input method in the remote
> process. The caret doesn't work in mash because the ash process can't
> see the browser's input method.
> 
> Add support for a11y to observe the input method objects created by the
> browser's input method driver / bridge. Explicitly forward the
> text caret bounds to ash, which then updates the highlight.
> 
> Bug:  867110 
> Test: added to ash_unittests and browser_tests
> Change-Id: I27058e091ad5a597de4ab644654df8d535e95b72
> Reviewed-on: https://chromium-review.googlesource.com/1151897
> Commit-Queue: James Cook <jamescook@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578473}

TBR=jamescook@chromium.org,xiyuan@chromium.org,tsepez@chromium.org

Change-Id: I8d2ed52affae2063d705f1ded9a2eca5dfdbda13
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  867110 
Reviewed-on: https://chromium-review.googlesource.com/1152593
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578557}
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/ash/accessibility/accessibility_controller.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/ash/accessibility/accessibility_controller.h
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/ash/accessibility/accessibility_highlight_controller.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/ash/accessibility/accessibility_highlight_controller.h
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/ash/accessibility/accessibility_highlight_controller_unittest.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/ash/display/window_tree_host_manager.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/ash/public/interfaces/accessibility_controller.mojom
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/ash/shell.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/ash/shell.h
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/chromeos/BUILD.gn
[delete] https://crrev.com/d488ff4568c7c38d4927046f278c9d3437aeebae/chrome/browser/chromeos/accessibility/accessibility_input_method_observer.cc
[delete] https://crrev.com/d488ff4568c7c38d4927046f278c9d3437aeebae/chrome/browser/chromeos/accessibility/accessibility_input_method_observer.h
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/chromeos/accessibility/accessibility_manager.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/chromeos/accessibility/accessibility_manager.h
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/ui/ash/accessibility/accessibility_controller_client_unittest.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_util.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/ui/views/ime_driver/ime_driver_mus.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/ui/views/ime_driver/ime_driver_mus.h
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc
[modify] https://crrev.com/ee3b12babcf98ab2020c827de2fb1e313e0102e5/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.h
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 27 

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

commit c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2
Author: James Cook <jamescook@chromium.org>
Date: Fri Jul 27 21:00:56 2018

Reland "chromeos: Support a11y caret highlight in shortcut viewer app and mash"

This is a reland of 2094f0159a03dc63bb8a34ddcf68d46d1a7c5355

Reason for previous revert: viz_browser_tests failing on chromium.memory/Linux Chromium OS ASan LSan Tests
https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/28406

The test wasn't cleaning up a test-only observer, so it was accessing invalid
memory during the test shutdown spin of the message loop.

Original change's description:
> chromeos: Support a11y caret highlight in shortcut viewer app and mash
>
> The existing code updates the text input caret highlight by observing
> input method state. This works in classic ash because the input method
> is shared globally across ash and browser code. This doesn't work with
> remote apps because ash can't see the input method in the remote
> process. The caret doesn't work in mash because the ash process can't
> see the browser's input method.
>
> Add support for a11y to observe the input method objects created by the
> browser's input method driver / bridge. Explicitly forward the
> text caret bounds to ash, which then updates the highlight.
>
> Bug:  867110 
> Test: added to ash_unittests and browser_tests
> Change-Id: I27058e091ad5a597de4ab644654df8d535e95b72
> Reviewed-on: https://chromium-review.googlesource.com/1151897
> Commit-Queue: James Cook <jamescook@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578473}

TBR=tsepez@chromium.org

Bug:  867110 
Change-Id: I47b3b1af6d0bbc7183bbc7248492cb4d6ec3feca
Reviewed-on: https://chromium-review.googlesource.com/1152373
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578779}
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/ash/accessibility/accessibility_controller.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/ash/accessibility/accessibility_controller.h
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/ash/accessibility/accessibility_highlight_controller.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/ash/accessibility/accessibility_highlight_controller.h
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/ash/accessibility/accessibility_highlight_controller_unittest.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/ash/display/window_tree_host_manager.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/ash/public/interfaces/accessibility_controller.mojom
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/ash/shell.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/ash/shell.h
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/chromeos/BUILD.gn
[add] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/chromeos/accessibility/accessibility_input_method_observer.cc
[add] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/chromeos/accessibility/accessibility_input_method_observer.h
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/chromeos/accessibility/accessibility_manager.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/chromeos/accessibility/accessibility_manager.h
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/chromeos/accessibility/accessibility_manager_browsertest.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/ui/ash/accessibility/accessibility_controller_client_unittest.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_util.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/ui/views/ime_driver/ime_driver_mus.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/ui/views/ime_driver/ime_driver_mus.h
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc
[modify] https://crrev.com/c9d6a54133794e1c11bd1b67a3d56c8af2c7bac2/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.h

Comment 5 by jamescook@chromium.org, Aug 2

Status: Fixed (was: Started)
►

Sign in to add a comment