New issue
Advanced search Search tips

Issue 910792 link

Starred by 1 user
Status: Started
Owner:
aleventhal@chromium.org
Cc:
dtseng@chromium.org
gov...@chromium.org
lpalmaro@chromium.org
Components:
EstimatedDays: ----
NextAction: 2018-12-11
OS: Chrome
Pri: 2
Type: Bug-Regression
Team-Accessibility



Sign in to add a comment

ChromeVox announces 'default' after each button in settings

Project Member Reported by lprazdnik@chromium.org, Dec 1 
Version 72.0.3625.0 (Official Build) canary (64-bit)
Firmware version: Google_cave.7820.384.o
steps:
# Launch ChromeVox (Alt+CTRL+Z)
# Navigate to status tray > settings
# Use search+left/right to navigate through settings, particularly buttons
Expected: ChromeVox should announce button label, followed by "button". Example: "Main menu button"
Actual: ChromeVox announces "default" after announcing label of button. Example: "Main Menu, default button"
Reproduces in Canary

Comment 1 by lprazdnik@chromium.org, Dec 1

Labels: -Pri-3 Pri-2

Comment 2 by lprazdnik@chromium.org, Dec 1

Description: Show this description

Comment 3 by aleventhal@google.com, Dec 11

Labels: Hotlist-GoodFirstBug
Perhaps Blink should be less aggressive and only report default if there's more than one. It might want to look for a parent <form> element as well.

Comment 4 by aleventhal@google.com, Dec 11 

Here is where we currently determine whether something gets the default state:
AXLayoutObject::IsDefault()
https://cs.chromium.org/chromium/src/third_party/blink/renderer/modules/accessibility/ax_layout_object.cc?type=cs&q=IsDefault+file:accessi&sq=package:chromium&g=0&l=308

Comment 5 by dtseng@chromium.org, Dec 11

Labels: M-72
NextAction: 2018-12-11
Owner: aleventhal@chromium.org
This is a regression and leads to a fairly poor experience. Aaron, do you think you could resolve it before 72 goes to beta?

Comment 6 by aleventhal@google.com, Dec 12

Status: Started (was: Available)

Comment 7 by aleventhal@google.com, Dec 12

Labels: -Hotlist-GoodFirstBug

Comment 8 by aleventhal@chromium.org, Dec 13

Labels: Merge-Request-72 FoundIn-72
Requesting merge to 72 for CL about to land: https://chromium-review.googlesource.com/c/chromium/src/+/1374490
Project Member

Comment 9 by sheriffbot@chromium.org, Dec 13

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

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

Comment 10 by bugdroid1@chromium.org, Dec 13 

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

commit 74573ed6690f185544afd2591fe8312b8cf91258
Author: Aaron Leventhal <aleventhal@chromium.org>
Date: Thu Dec 13 22:19:12 2018

Only consider first submit button in a form as default

Test: with ChromeVox and chrome://settings
Bug: 910792
Change-Id: I8531ee913be0c62e177f207d2f1ef28d4b28d4f3
Reviewed-on: https://chromium-review.googlesource.com/c/1374490
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616454}
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/chrome/test/data/extensions/api_test/automation/tests/tabs/sanity_check.js
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/event/disabled-state-changed-expected-win.txt
[add] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/button-submit-expected-blink.txt
[add] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/button-submit.html
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-image-expected-auralinux.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-image-expected-blink.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-image-expected-win.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-submit-expected-android.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-submit-expected-auralinux.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-submit-expected-blink.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-submit-expected-mac.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-submit-expected-win.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-submit.html
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-types-expected-auralinux.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-types-expected-blink.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/content/test/data/accessibility/html/input-types-expected-win.txt
[modify] https://crrev.com/74573ed6690f185544afd2591fe8312b8cf91258/third_party/blink/renderer/modules/accessibility/ax_layout_object.cc

Comment 11 by dgagnon@google.com, Dec 14

Cc: gov...@chromium.org
If this is changing strings, we are past sting change freeze. If this is critical for release, please let us know and we can review further.

Comment 12 by aleventhal@chromium.org, Dec 14 

This doesn't change any strings.

Comment 13 by dgagnon@google.com, Dec 14

Labels: -Hotlist-Merge-Review -Merge-Review-72 Merge-Approved-72
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 14

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98899b699f32b64cf60e7882269d12971ddd8b15

commit 98899b699f32b64cf60e7882269d12971ddd8b15
Author: Aaron Leventhal <aleventhal@chromium.org>
Date: Fri Dec 14 21:15:52 2018

Only consider first submit button in a form as default

Test: with ChromeVox and chrome://settings
Bug: 910792
Change-Id: I8531ee913be0c62e177f207d2f1ef28d4b28d4f3
Reviewed-on: https://chromium-review.googlesource.com/c/1374490
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616454}(cherry picked from commit 74573ed6690f185544afd2591fe8312b8cf91258)
Reviewed-on: https://chromium-review.googlesource.com/c/1378640
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#367}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/chrome/test/data/extensions/api_test/automation/tests/tabs/sanity_check.js
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/event/disabled-state-changed-expected-win.txt
[add] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/button-submit-expected-blink.txt
[add] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/button-submit.html
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-image-expected-auralinux.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-image-expected-blink.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-image-expected-win.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-submit-expected-android.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-submit-expected-auralinux.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-submit-expected-blink.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-submit-expected-mac.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-submit-expected-win.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-submit.html
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-types-expected-auralinux.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-types-expected-blink.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/content/test/data/accessibility/html/input-types-expected-win.txt
[modify] https://crrev.com/98899b699f32b64cf60e7882269d12971ddd8b15/third_party/blink/renderer/modules/accessibility/ax_layout_object.cc
Project Member

Comment 15 by cr-audit...@appspot.gserviceaccount.com, Dec 19

Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 98899b699f32b64cf60e7882269d12971ddd8b15 was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required --
Project Member

Comment 16 by cr-audit...@appspot.gserviceaccount.com, Dec 19

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/98899b699f32b64cf60e7882269d12971ddd8b15

Commit: 98899b699f32b64cf60e7882269d12971ddd8b15
Author: aleventhal@chromium.org
Commiter: aleventhal@chromium.org
Date: 2018-12-14 21:15:52 +0000 UTC

Only consider first submit button in a form as default

Test: with ChromeVox and chrome://settings
Bug: 910792
Change-Id: I8531ee913be0c62e177f207d2f1ef28d4b28d4f3
Reviewed-on: https://chromium-review.googlesource.com/c/1374490
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616454}(cherry picked from commit 74573ed6690f185544afd2591fe8312b8cf91258)
Reviewed-on: https://chromium-review.googlesource.com/c/1378640
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#367}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Comment 17 by aleventhal@chromium.org, Dec 21 

I'm confused about the violation. 
In comment 13, @dgagnon added Merge-Approved-72
In comment 14, Bugdroid removed Merge-Approved-72

It was merged into 3626, which according to https://chromiumdash.appspot.com/branches is the correct branch for 72.

Comment 18 by dgagnon@google.com, Dec 21 

It appears that this was actually merged on the 13th but Merge-Approved-72 was not tagged until the 14th.

Comment 19 by aleventhal@chromium.org, Dec 21 

@dgagnon, how so? 
- Merged into the main trunk on the 13th (bugdroid comment 10).
- Tagged with Merge-Approved-72 on the 14th (comment 13)
- Merged into 3626 on the 14th (bugdroid comment 14)

And in any case, I clearly remember only merging into 72 after checking the bug for approval.

Comment 20 by dgagnon@google.com, Dec 21 

Hmm, yes, even the link in comment #15 shows it was merged into 3626 on the 14th.
I'm not sure why the cr-audit-commits@ bot marked it as a violation.
►

Sign in to add a comment