New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 778859 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression
Team-Accessibility



Sign in to add a comment

[A11y Assessment - NTP] search menu button (where security chip goes) labeled as button

Project Member Reported by leberly@chromium.org, Oct 26 2017

Issue description

Google Chrome 64.0.3250.0 (Official Build) canary (64-bit) (cohort: 64-Bit)
Windows 10 Enterprise Version 1607 Build 14393.1770
NVDA 2017.3
JAWS 2018.1710.42 private preview release

JAWS and NVDA both read NTP security button as being unlabeled, JAWS does not read corresponding dialog while NVDA does. Here's steps to repro:

# Launch Chrome and JAWS or NVDA, open New Tab Page NTP 
# Press F6 to get to the omnibar 
# Shift-tab once to get to the magnifying glass icon. Listen to what's spoken

JAWS: "button menu Press space to activate the menu, then navigate with arrow keys"
NVDA: "menu button" 

# press space bar to invoke the dialog. Dialog appears and contains "You're viewing a secure Google Chrome page" 

JAWS: says "dialog To navigate use T A B" but tab nor arrow keys actually read the contents of the dialog. User had no way to access it nor hear the contents of that dialog.  

NVDA: says "dialog  You're viewing a secure Google Chrome page". There is no way to access the dialog with the arrow keys but since the content of the dialog is read, the user gets all the info anyway. 



 
 
Labels: -Pri-2 Pri-1
Additional Canary test: If I turn off all AT software and just use the keyboard, I can reproduce. I press F6, shift-tab, then space to get the dialog to appear. Normally, shift tab should take me to the Home button. 

Bumping to priority 1 since this is a regression. 

Other non-working builds: 
Does reproduce in Dev - Google Chrome	63.0.3239.18 (Official Build) dev (64-bit) (cohort: Dev)

Does reproduce in Beta - Google Chrome	63.0.3239.18 (Official Build) beta (64-bit) (cohort: Beta)

Last known working build: 
Does not reproduce in Stable - Google Chrome 61.0.3163.100 (Official Build) (64-bit) (cohort: Stable)
You are probably looking for a change made after 506308 (known good), but no later than 506309 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/ad84e4b20f836797b8e4a842b8d5e70f7531ba7c..8d15c77dedd4725f6ac4ec2af51826b419d8bfe2
Labels: -Type-Bug ReleaseBlock-Stable M-63 Type-Bug-Regression
Owner: spqc...@chromium.org
Status: Assigned (was: Available)
Status: Started (was: Assigned)
M63 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge to M63 ASAP. Thank you.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 8 2017

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

commit 825fa0da1e43848312b9f5edd5aba5e6d8290aeb
Author: Sarah Chan <spqchan@chromium.org>
Date: Wed Nov 08 22:50:05 2017

[Views] AX Fix for the Omnibox Search Icon

The search icon is incorrectly labeled as a button
by screen readers.

To fix this, set the LocationIconView's accessibility
role to be NONE if it's displaying the search icon.

Bug:  778859 
Change-Id: I0504c72009672d7617e465265035a13059535f50
Reviewed-on: https://chromium-review.googlesource.com/754041
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514982}
[modify] https://crrev.com/825fa0da1e43848312b9f5edd5aba5e6d8290aeb/chrome/browser/ui/views/location_bar/location_icon_view.cc

Comment 8 by gov...@chromium.org, Nov 10 2017

How is this change looking in Canary? If it looks good and safe to merge, pls request a merge to M63. Thank you.
Labels: Merge-Request-63
Project Member

Comment 10 by sheriffbot@chromium.org, Nov 13 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The fix is low risk and had been tested
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comment #11. Please merge today before 4:00 PM PT so we can take it in for this week Beta release. Thank you.
M63 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge  into the release branch ASAP. Thank you.



Project Member

Comment 14 by bugdroid1@chromium.org, Nov 13 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6935289faf1d80a164503b8b9bf02bc854a514e4

commit 6935289faf1d80a164503b8b9bf02bc854a514e4
Author: spqchan <spqchan@chromium.org>
Date: Mon Nov 13 21:34:08 2017

[Views] AX Fix for the Omnibox Search Icon

The search icon is incorrectly labeled as a button
by screen readers.

To fix this, set the LocationIconView's accessibility
role to be NONE if it's displaying the search icon.

(cherry picked from commit 825fa0da1e43848312b9f5edd5aba5e6d8290aeb)

Bug:  778859 
Change-Id: I0504c72009672d7617e465265035a13059535f50
Reviewed-on: https://chromium-review.googlesource.com/754041
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#514982}
Reviewed-on: https://chromium-review.googlesource.com/767087
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#469}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/6935289faf1d80a164503b8b9bf02bc854a514e4/chrome/browser/ui/views/location_bar/location_icon_view.cc

Status: Fixed (was: Started)
Status: Available (was: Fixed)
Hello,

Just letting you know that this is NOT fixed in Canary and the state has not changed for me in Beta or Dev. If the proposed fix in M63 is the same as what is in Canary now, then this issue is not resolved. 

Steps to repro new state in Canary: 
# Launch JAWS or NVDA
# Open Chrome to the NTP 
# Press F6 to get into the Omnibox 
# Press shift + tab to put focus on the search icon
Expected: icon is labeled, screen reader reads label  
Actual: icon is not labeled 
With JAWS, it says nothing when focus is on the icon
With NVDA, it says "pane" when focus is on the icon. 

Please let me know when I may test the change proposed to get into M63. 
Setting from fixed to available. 

Versions tested:
Google Chrome	64.0.3267.1 (Official Build) canary SyzyASan (32-bit) (cohort: ASAN)
Google Chrome	63.0.3239.40 (Official Build) beta (64-bit) (cohort: Beta)
Google Chrome	64.0.3260.2 (Official Build) dev (64-bit) (cohort: Dev)

Thanks,

Laura 
Hey Laura,

You can't interact with the icon, so I removed it from the screenreader so it cannot be focused. Are you sure you're focused on the icon when it says "pane" on NVDA?

What is the expected behavior?

Note: When I say icon, I meant the "search icon", not when there's an label and the icon is clickable. I can change that behavior so the search icon gives the text "label" though

Labels: Needs-Feedback
Adding Needs-Feedback as per # 17
Cc: lpalmaro@chromium.org
Google Chrome	64.0.3268.0 (Official Build) canary (64-bit) (cohort: Clang-64)
Google Chrome	63.0.3239.40 (Official Build) beta (64-bit) (cohort: Beta)
NVDA 2017.3

Hello,

I retested with today's Canary and in today's 63 in the Beta channel. The result is that this test is still failing - I am still able to select the icon which is the root of the problem. I can repro the behavior I described above where the icon is selected in the tab order with keyboard-only and with NVDA screen reader. In 63 it is also still saying "menu button".  

I'm using these steps:
# launch Chrome and screen reader if applicable 
# Press F6 to put the focus in the Omnibox
# Use shift-tab to move backwards through the tab order. 
# Note that I can select the search magnifying glass icon, the problem behavior. 
# Press shift-tab again to put focus on the refresh button. 

I agree that my expected behavior is that you are not able to focus on the search icon in the Omnibox at all since it doesn't have a function beyond being decorative. It's also my expected behavior since this is how things used to be before this bug. 

Given how important this bug is becoming, being a potential release blocker and such, I'm plussing in my PgM, Laura Allen, just to give her a heads up and to see if she has any additional comments on the expected behavior.  

Please let me know if you have further questions. 

Thanks,

Laura 




This should not block the beta refresh. Will look at the bug this week
I'm having trouble reproducing the "menu button" in comment #20. What Windows version are you on?

Just to clarify, the AX behavior for the left icon should be as follows:

When the icon is interactive and not showing the search icon, it should give "menu button"
However, when it's just displaying the search icon, it should not be interactive at all.




M63 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge to M63 ASAP. Thank you.
Google Chrome	64.0.3271.0 (Official Build) canary (64-bit) (cohort: Clang-64)
Windows 10 Enterprise Version 1607 Build 14393.1770
NVDA 2017.3
JAWS 2018.1710.42 private preview release

Hello,

I agree that it shouldn't be focusable but I can reliably reproduce focusing on it. When would this button be interactive? 

Also, "button menu" is not a good solution because that means that there is an unlabeled button which is a very poor user experience. 

I am on Windows 10 and I retested again with the latest Canary. Here are the exact steps I used:

keyboard only test
# launch Chrome
# Press F6 to put the focus in the Omnibox
# Use shift-tab to move backwards through the tab order. 
# Note that I can select the search magnifying glass icon, the problem behavior. There is also no focus ring on this button so it looks like focus just disappeared. I know that focus is there, however, because 
# Press shift-tab again to put focus on the refresh button. 

Repeat with JAWS and NVDA screen readers:
JAWS says nothing while focusing on this button. It shouldn't be able to focus on it at all. 
NVDA says "pane" 

Please let me know if you have more questions. 

Thanks,

Laura 


Awesome, thanks for the clarification. I will have a fix up by tonight
Thank you  spqchan@. Please update the bug once cl listed #26 landed/baked/verified in Canary.
Project Member

Comment 28 by bugdroid1@chromium.org, Nov 21 2017

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

commit a43aaf3011ef872121b6d81639290197a6ed4ea7
Author: Sarah Chan <spqchan@chromium.org>
Date: Tue Nov 21 01:52:45 2017

[Views] Fix LocationIconView focusable behavior

Currently the LocationIconView can be selected as
part of keyboard access when the search icon is
displayed (and therefore not clickable).

This CL fixes it by setting its FocusBehavior to
never when the search icon is displayed.

Bug:  778859 
Change-Id: If0d57efed6d3ecfd2f0ad01cc1fd105d65feb18a
Reviewed-on: https://chromium-review.googlesource.com/777966
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518053}
[modify] https://crrev.com/a43aaf3011ef872121b6d81639290197a6ed4ea7/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/a43aaf3011ef872121b6d81639290197a6ed4ea7/chrome/browser/ui/views/location_bar/location_icon_view.cc
[modify] https://crrev.com/a43aaf3011ef872121b6d81639290197a6ed4ea7/chrome/browser/ui/views/location_bar/location_icon_view.h
[modify] https://crrev.com/a43aaf3011ef872121b6d81639290197a6ed4ea7/chrome/browser/ui/views/location_bar/location_icon_view_browsertest.cc

How is the change listed at #28 looking in canary?
It's not yet available in Canary
Labels: Merge-Request-63
Verified on Canary and it looks good
Project Member

Comment 32 by sheriffbot@chromium.org, Nov 22 2017

Labels: -Merge-Request-63 Merge-Review-63
This bug requires manual review: We are only 12 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The CL in #28 low risk and tested
Google Chrome 64.0.3275.0 (Official Build) canary (64-bit) (cohort: Clang-64)
Windows 10 Enterprise Version 1607 Build 14393.1770
NVDA 2017.3
JAWS 2018.1710.42 private preview release

Yes, I verified in Canary as well and this looks great. If you need further testing in the near future, please reach out to lpalmaro@, copied here, as I will be out of the office intermittently. 

Steps I used to verify:

Keyboard only:
# Launch Chrome Canary NTP
# Press F6 to get into the Omnibox
# Press shift + tab
Focus goes to the refresh button as expected. There is button over the magnifying glass.

I repeated these steps with screen readers and found the same behavior. There are no more incorrect verbalizations since there is no longer a button over the magnifying glass.

Thanks!

Thanks,

Laura E 
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comments #31, #33 and #34.
Project Member

Comment 36 by sheriffbot@chromium.org, Nov 27 2017

Cc: gov...@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 37 by bugdroid1@chromium.org, Nov 27 2017

Labels: -merge-approved-63
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1a6ddf4c5c80fb68d4e0c7255783ddb1bd4e31d

commit a1a6ddf4c5c80fb68d4e0c7255783ddb1bd4e31d
Author: spqchan <spqchan@chromium.org>
Date: Mon Nov 27 20:08:31 2017

[Views] Fix LocationIconView focusable behavior

Currently the LocationIconView can be selected as
part of keyboard access when the search icon is
displayed (and therefore not clickable).

This CL fixes it by setting its FocusBehavior to
never when the search icon is displayed.

(cherry picked from commit a43aaf3011ef872121b6d81639290197a6ed4ea7)

Bug:  778859 
Change-Id: If0d57efed6d3ecfd2f0ad01cc1fd105d65feb18a
Reviewed-on: https://chromium-review.googlesource.com/777966
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#518053}
Reviewed-on: https://chromium-review.googlesource.com/791519
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#573}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/a1a6ddf4c5c80fb68d4e0c7255783ddb1bd4e31d/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/a1a6ddf4c5c80fb68d4e0c7255783ddb1bd4e31d/chrome/browser/ui/views/location_bar/location_icon_view.cc
[modify] https://crrev.com/a1a6ddf4c5c80fb68d4e0c7255783ddb1bd4e31d/chrome/browser/ui/views/location_bar/location_icon_view.h
[modify] https://crrev.com/a1a6ddf4c5c80fb68d4e0c7255783ddb1bd4e31d/chrome/browser/ui/views/location_bar/location_icon_view_browsertest.cc

Status: Fixed (was: Available)
Labels: win-a11y

Sign in to add a comment