New issue
Advanced search Search tips

Issue 878072 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 28
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug
Team-Accessibility

Blocking:
issue 876315
issue 876605
issue 877208



Sign in to add a comment

Chrome is enabling accessibility support on Windows too often

Project Member Reported by dmazz...@chromium.org, Aug 27

Issue description

There are two ways Chrome detects the presence of assistive technology in order to enable accessibility support on Windows:

1. We fire an EVENT_SYSTEM_ALERT event on a "honeypot" window, if it's queried we enable accessibility.
2. We check for QueryService to be called with IID_IAccessible2, since that's only used by screen readers.

Option #1 seems to be catching many users who don't actually need accessibility enabled. We need to make it a bit more strict and not enable accessibility unless additional APIs are called too.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 27

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

commit 641225c2e54ccd63bfdb6177f734d5727adba939
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Mon Aug 27 23:00:48 2018

Enable accessibility less often on Windows

There are two ways Chrome detects the presence of assistive
technology in order to enable accessibility support on Windows:

1. We fire an EVENT_SYSTEM_ALERT event on a "honeypot" window,
   if it's queried we enable accessibility.
2. We check for QueryService to be called with IID_IAccessible2,
   since that's only used by screen readers.

Option #1 seems to be catching many users who don't actually need
accessibility enabled. This change skips enabling accessibility
unless get_accName is also called.

Bug:  878072 ,877208
TBR: jochen@chromium.org
Change-Id: I5f36c632822faa607d301d543b7ed8d630e835fe
Reviewed-on: https://chromium-review.googlesource.com/1192062
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586470}
[modify] https://crrev.com/641225c2e54ccd63bfdb6177f734d5727adba939/content/browser/accessibility/browser_accessibility_manager_win.cc
[modify] https://crrev.com/641225c2e54ccd63bfdb6177f734d5727adba939/content/browser/accessibility/browser_accessibility_manager_win.h
[modify] https://crrev.com/641225c2e54ccd63bfdb6177f734d5727adba939/content/browser/renderer_host/legacy_render_widget_host_win.cc
[modify] https://crrev.com/641225c2e54ccd63bfdb6177f734d5727adba939/ui/accessibility/platform/ax_platform_node_win.cc
[modify] https://crrev.com/641225c2e54ccd63bfdb6177f734d5727adba939/ui/accessibility/platform/ax_platform_node_win.h

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 27

Labels: merge-merged-3534
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/468e5d3df4abc31380a7c8d4816033bd078f2270

commit 468e5d3df4abc31380a7c8d4816033bd078f2270
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Mon Aug 27 23:02:07 2018

Merge to Canary branch 3534: Enable accessibility less often on Windows

There are two ways Chrome detects the presence of assistive
technology in order to enable accessibility support on Windows:

1. We fire an EVENT_SYSTEM_ALERT event on a "honeypot" window,
   if it's queried we enable accessibility.
2. We check for QueryService to be called with IID_IAccessible2,
   since that's only used by screen readers.

Option #1 seems to be catching many users who don't actually need
accessibility enabled. This change skips enabling accessibility
unless get_accName is also called.

Bug:  878072 ,877208
TBR: jochen@chromium.org
Change-Id: I5f36c632822faa607d301d543b7ed8d630e835fe
Reviewed-on: https://chromium-review.googlesource.com/1192062
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#586470}(cherry picked from commit 161077958aaf7eaba804cd1c27510ebc10bf00dc)
Reviewed-on: https://chromium-review.googlesource.com/1192117
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/branch-heads/3534@{#8}
Cr-Branched-From: 5c1309f3276665c501e62f79a3bfb9244b3dca26-refs/heads/master@{#586175}
[modify] https://crrev.com/468e5d3df4abc31380a7c8d4816033bd078f2270/content/browser/accessibility/browser_accessibility_manager_win.cc
[modify] https://crrev.com/468e5d3df4abc31380a7c8d4816033bd078f2270/content/browser/accessibility/browser_accessibility_manager_win.h
[modify] https://crrev.com/468e5d3df4abc31380a7c8d4816033bd078f2270/content/browser/renderer_host/legacy_render_widget_host_win.cc
[modify] https://crrev.com/468e5d3df4abc31380a7c8d4816033bd078f2270/ui/accessibility/platform/ax_platform_node_win.cc
[modify] https://crrev.com/468e5d3df4abc31380a7c8d4816033bd078f2270/ui/accessibility/platform/ax_platform_node_win.h

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 28

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df50ae4d2550f29f625ddac046e0ed65f398136e

commit df50ae4d2550f29f625ddac046e0ed65f398136e
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Tue Aug 28 18:00:51 2018

Merge to M69: Enable accessibility less often on Windows

There are two ways Chrome detects the presence of assistive
technology in order to enable accessibility support on Windows:

1. We fire an EVENT_SYSTEM_ALERT event on a "honeypot" window,
   if it's queried we enable accessibility.
2. We check for QueryService to be called with IID_IAccessible2,
   since that's only used by screen readers.

Option #1 seems to be catching many users who don't actually need
accessibility enabled. This change skips enabling accessibility
unless get_accName is also called.

Bug:  878072 ,877208
TBR: jochen@chromium.org
Change-Id: I5f36c632822faa607d301d543b7ed8d630e835fe
Reviewed-on: https://chromium-review.googlesource.com/1192062
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#586470}(cherry picked from commit 641225c2e54ccd63bfdb6177f734d5727adba939)
Reviewed-on: https://chromium-review.googlesource.com/1193852
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#830}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/df50ae4d2550f29f625ddac046e0ed65f398136e/content/browser/accessibility/browser_accessibility_manager_win.cc
[modify] https://crrev.com/df50ae4d2550f29f625ddac046e0ed65f398136e/content/browser/accessibility/browser_accessibility_manager_win.h
[modify] https://crrev.com/df50ae4d2550f29f625ddac046e0ed65f398136e/content/browser/renderer_host/legacy_render_widget_host_win.cc
[modify] https://crrev.com/df50ae4d2550f29f625ddac046e0ed65f398136e/ui/accessibility/platform/ax_platform_node_win.cc
[modify] https://crrev.com/df50ae4d2550f29f625ddac046e0ed65f398136e/ui/accessibility/platform/ax_platform_node_win.h

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 21

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

commit 33d3164a86645b1620f134b29c3836553ae04792
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Fri Sep 21 17:02:10 2018

Ensure Windows accessibility is enabled properly.

http://crrev.com/c/1192062 made it so that we don't enable accessibility
as often. The idea was to only enable accessibility if we get a call to
IAccessible2, or to both get_accName and a response to our alert message
on a honeypot window.

However, this only worked if we constructed at least one
BrowserAccessibilityManager. In cases where the external client
never explored and discovered a BrowserAccessibilityManager, the
IAccessible2UsageObserver wasn't registered yet.

As a fix, move the IAccessible2UsageObserver code to part of
BrowserAccessibilityStateImpl.

Bug:  878072 
Change-Id: Iadcbd032fa1d6f635bbb99c1e130d384efe0d9b1
Reviewed-on: https://chromium-review.googlesource.com/1234267
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593233}
[modify] https://crrev.com/33d3164a86645b1620f134b29c3836553ae04792/content/browser/accessibility/browser_accessibility_manager_win.cc
[modify] https://crrev.com/33d3164a86645b1620f134b29c3836553ae04792/content/browser/accessibility/browser_accessibility_manager_win.h
[modify] https://crrev.com/33d3164a86645b1620f134b29c3836553ae04792/content/browser/accessibility/browser_accessibility_state_impl.cc
[modify] https://crrev.com/33d3164a86645b1620f134b29c3836553ae04792/content/browser/accessibility/browser_accessibility_state_impl.h
[modify] https://crrev.com/33d3164a86645b1620f134b29c3836553ae04792/content/browser/accessibility/browser_accessibility_state_impl_mac.mm
[modify] https://crrev.com/33d3164a86645b1620f134b29c3836553ae04792/content/browser/accessibility/browser_accessibility_state_impl_win.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 24

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

commit 21472615ea147c234988c27bb3e3f7d86ea79f33
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Mon Sep 24 17:05:55 2018

Revert "Ensure Windows accessibility is enabled properly."

This reverts commit 33d3164a86645b1620f134b29c3836553ae04792.

Reason for revert: Breaks NVDA / JAWS

Original change's description:
> Ensure Windows accessibility is enabled properly.
> 
> http://crrev.com/c/1192062 made it so that we don't enable accessibility
> as often. The idea was to only enable accessibility if we get a call to
> IAccessible2, or to both get_accName and a response to our alert message
> on a honeypot window.
> 
> However, this only worked if we constructed at least one
> BrowserAccessibilityManager. In cases where the external client
> never explored and discovered a BrowserAccessibilityManager, the
> IAccessible2UsageObserver wasn't registered yet.
> 
> As a fix, move the IAccessible2UsageObserver code to part of
> BrowserAccessibilityStateImpl.
> 
> Bug:  878072 
> Change-Id: Iadcbd032fa1d6f635bbb99c1e130d384efe0d9b1
> Reviewed-on: https://chromium-review.googlesource.com/1234267
> Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
> Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#593233}

TBR=dmazzoni@chromium.org,aleventhal@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  878072 
Change-Id: I482cb7e3fa19f5f6357c4bc28f44cb20272142cd
Reviewed-on: https://chromium-review.googlesource.com/1240473
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593574}
[modify] https://crrev.com/21472615ea147c234988c27bb3e3f7d86ea79f33/content/browser/accessibility/browser_accessibility_manager_win.cc
[modify] https://crrev.com/21472615ea147c234988c27bb3e3f7d86ea79f33/content/browser/accessibility/browser_accessibility_manager_win.h
[modify] https://crrev.com/21472615ea147c234988c27bb3e3f7d86ea79f33/content/browser/accessibility/browser_accessibility_state_impl.cc
[modify] https://crrev.com/21472615ea147c234988c27bb3e3f7d86ea79f33/content/browser/accessibility/browser_accessibility_state_impl.h
[modify] https://crrev.com/21472615ea147c234988c27bb3e3f7d86ea79f33/content/browser/accessibility/browser_accessibility_state_impl_mac.mm
[modify] https://crrev.com/21472615ea147c234988c27bb3e3f7d86ea79f33/content/browser/accessibility/browser_accessibility_state_impl_win.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 26

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

commit 190a6cd9937c76e23fff6a66a50bea517a0e608e
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Wed Sep 26 11:24:08 2018

Reland "Ensure Windows accessibility is enabled properly."

Original: http://crrev.com/c/1234267
Reverted: http://crrev.com/c/1240473

Fix: see diff between patchset 1 and patchset 3. Really
stupid error, I introduced PlatformInitialize() while
cleaning up the code and didn't call it from the
constructor. Now manually verified to work with JAWS
and NVDA.

http://crrev.com/c/1192062 made it so that we don't enable accessibility
as often. The idea was to only enable accessibility if we get a call to
IAccessible2, or to both get_accName and a response to our alert message
on a honeypot window.

However, this only worked if we constructed at least one
BrowserAccessibilityManager. In cases where the external client
never explored and discovered a BrowserAccessibilityManager, the
IAccessible2UsageObserver wasn't registered yet.

As a fix, move the IAccessible2UsageObserver code to part of
BrowserAccessibilityStateImpl.

Bug:  878072 
Change-Id: I3f85f1ef3b9f730d40a5bf90d01ec6714dce66e3
Reviewed-on: https://chromium-review.googlesource.com/1244141
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594281}
[modify] https://crrev.com/190a6cd9937c76e23fff6a66a50bea517a0e608e/content/browser/accessibility/browser_accessibility_manager_win.cc
[modify] https://crrev.com/190a6cd9937c76e23fff6a66a50bea517a0e608e/content/browser/accessibility/browser_accessibility_manager_win.h
[modify] https://crrev.com/190a6cd9937c76e23fff6a66a50bea517a0e608e/content/browser/accessibility/browser_accessibility_state_impl.cc
[modify] https://crrev.com/190a6cd9937c76e23fff6a66a50bea517a0e608e/content/browser/accessibility/browser_accessibility_state_impl.h
[modify] https://crrev.com/190a6cd9937c76e23fff6a66a50bea517a0e608e/content/browser/accessibility/browser_accessibility_state_impl_mac.mm
[modify] https://crrev.com/190a6cd9937c76e23fff6a66a50bea517a0e608e/content/browser/accessibility/browser_accessibility_state_impl_win.cc

Sign in to add a comment