DCHECK: WebUI must set accessible name on roles with name=prohibited [chromium/src : main]

0 views
Skip to first unread message

Aaron Leventhal (Gerrit)

unread,
Jul 1, 2024, 12:45:13 PM (2 days ago) Jul 1
to Chris Harrelson, Zijie He, Alex Keng, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, (Julie)Jeongeun Kim, Kevin Babbitt, Nektarios Paisios, kinuko...@chromium.org, jmedle...@chromium.org, cros-print...@google.com, chromium-a...@chromium.org, extension...@chromium.org, print-rev...@chromium.org, filesapp...@chromium.org, cros-setti...@google.com, croissant-...@chromium.org, rginda...@chromium.org, michaelpg+wa...@chromium.org, alemat...@chromium.org, dtseng+c...@chromium.org, blundell+...@chromium.org, oshima...@chromium.org, rrsilva+wat...@google.com, shannc...@chromium.org, anastas...@google.com, katie...@chromium.org, fuchsia...@chromium.org, steimel+watch...@chromium.org, devtools-re...@chromium.org, feature-me...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, aleventhal...@chromium.org, blink-...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, hirokisa...@chromium.org, josiah...@chromium.org, kyungjunle...@google.com, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org
Attention needed from Chris Harrelson

Aaron Leventhal added 1 comment

Patchset-level comments
File-level comment, Patchset 35 (Latest):
Aaron Leventhal . resolved

Hi Chris, PTAL

Open in Gerrit

Related details

Attention is currently required from:
  • Chris Harrelson
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ic59c0ca40dfedc9615f377c9a46c737557a38671
Gerrit-Change-Number: 4035272
Gerrit-PatchSet: 35
Gerrit-Owner: Aaron Leventhal <aleve...@chromium.org>
Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
Gerrit-Reviewer: Chris Harrelson <chri...@chromium.org>
Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alex Keng <shi...@microsoft.com>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
Gerrit-CC: Zijie He <zij...@google.com>
Gerrit-Attention: Chris Harrelson <chri...@chromium.org>
Gerrit-Comment-Date: Mon, 01 Jul 2024 16:45:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Chris Harrelson (Gerrit)

unread,
Jul 1, 2024, 1:59:29 PM (2 days ago) Jul 1
to Aaron Leventhal, Zijie He, Alex Keng, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, (Julie)Jeongeun Kim, Kevin Babbitt, Nektarios Paisios, kinuko...@chromium.org, jmedle...@chromium.org, cros-print...@google.com, chromium-a...@chromium.org, extension...@chromium.org, print-rev...@chromium.org, filesapp...@chromium.org, cros-setti...@google.com, croissant-...@chromium.org, rginda...@chromium.org, michaelpg+wa...@chromium.org, alemat...@chromium.org, dtseng+c...@chromium.org, blundell+...@chromium.org, oshima...@chromium.org, rrsilva+wat...@google.com, shannc...@chromium.org, anastas...@google.com, katie...@chromium.org, fuchsia...@chromium.org, steimel+watch...@chromium.org, devtools-re...@chromium.org, feature-me...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, aleventhal...@chromium.org, blink-...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, hirokisa...@chromium.org, josiah...@chromium.org, kyungjunle...@google.com, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org
Attention needed from Aaron Leventhal

Chris Harrelson added 8 comments

File third_party/blink/renderer/modules/accessibility/ax_node_object.cc
Line 2173, Patchset 35 (Latest): // Expose <kbd> as a group, but only if it has a title.
Chris Harrelson . unresolved

Is this behavior in a specification? If so, please link to it.

Line 4750, Patchset 35 (Latest): // Do not use tooltip text for the name of an unfocusable node with a
Chris Harrelson . unresolved

Link to specification requiring this. (is it the one linked on line 4749?)

File third_party/blink/renderer/modules/accessibility/ax_object.cc
Line 4448, Patchset 35 (Latest): // happens correctly, the below line CanSetFocusAttribute() catch this case.
Chris Harrelson . unresolved
```suggestion
  // happens correctly, the below line CanSetFocusAttribute() catches this case.
```
Line 4456, Patchset 35 (Latest): // TODO(accessibility) Test to see whether the following content works in
Chris Harrelson . unresolved

Is there a plan to do that soon? A tracking bug?

Line 4471, Patchset 35 (Latest): // TODO(accessibility) Fix WebUI and remove this.
Chris Harrelson . unresolved

Is there a plan to do that soon? A tracking bug?

File third_party/blink/renderer/platform/runtime_enabled_features.json5
Line 257, Patchset 35 (Latest): name: "AccessibilityProhibitedNames",
Chris Harrelson . unresolved

What's the goal of making this experimental instead of test?

File third_party/blink/web_tests/accessibility/aria-hidden-with-elements-expected.txt
Line 25, Patchset 35 (Latest):FAIL child_4.isIgnored should be false. Was true.
Chris Harrelson . unresolved

Why is this now failing?

File third_party/blink/web_tests/accessibility/computed-name-expected.txt
Line 1, Patchset 35 (Latest):CONSOLE ERROR: An accessible name was placed on a prohibited role. This causes inconsistent behavior in screen readers. For example, <div aria-label="foo"> is invalid as it is not accessible in every screen reader, because they expect only to read the inner contents of certain types of containers. Please add a valid role or put the name on a different object. As a repair technique, the browser will place the prohibited name in the accessible description field. To learn more, see the section 'Roles which cannot be named' in the ARIA specification at https://w3c.github.io/aria/#namefromprohibited.
Chris Harrelson . unresolved

This is hard to fix?

Open in Gerrit

Related details

Attention is currently required from:
  • Aaron Leventhal
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ic59c0ca40dfedc9615f377c9a46c737557a38671
    Gerrit-Change-Number: 4035272
    Gerrit-PatchSet: 35
    Gerrit-Owner: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Reviewer: Chris Harrelson <chri...@chromium.org>
    Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alex Keng <shi...@microsoft.com>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
    Gerrit-CC: Zijie He <zij...@google.com>
    Gerrit-Attention: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Comment-Date: Mon, 01 Jul 2024 17:59:17 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Aaron Leventhal (Gerrit)

    unread,
    Jul 1, 2024, 7:07:51 PM (2 days ago) Jul 1
    to Chris Harrelson, Zijie He, Alex Keng, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, (Julie)Jeongeun Kim, Kevin Babbitt, Nektarios Paisios, blink-revie...@chromium.org, kinuko...@chromium.org, jmedle...@chromium.org, cros-print...@google.com, chromium-a...@chromium.org, extension...@chromium.org, print-rev...@chromium.org, filesapp...@chromium.org, cros-setti...@google.com, croissant-...@chromium.org, rginda...@chromium.org, michaelpg+wa...@chromium.org, alemat...@chromium.org, dtseng+c...@chromium.org, blundell+...@chromium.org, oshima...@chromium.org, rrsilva+wat...@google.com, shannc...@chromium.org, anastas...@google.com, katie...@chromium.org, fuchsia...@chromium.org, steimel+watch...@chromium.org, devtools-re...@chromium.org, feature-me...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, aleventhal...@chromium.org, blink-...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, hirokisa...@chromium.org, josiah...@chromium.org, kyungjunle...@google.com, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org
    Attention needed from Chris Harrelson

    Aaron Leventhal added 8 comments

    File third_party/blink/renderer/modules/accessibility/ax_node_object.cc
    Line 2173, Patchset 35: // Expose <kbd> as a group, but only if it has a title.
    Chris Harrelson . resolved

    Is this behavior in a specification? If so, please link to it.

    Aaron Leventhal

    Removing this, because the issue in the interop test was addressed in the linked github issue.

    Line 4750, Patchset 35: // Do not use tooltip text for the name of an unfocusable node with a
    Chris Harrelson . unresolved

    Link to specification requiring this. (is it the one linked on line 4749?)

    Aaron Leventhal

    Done.

    File third_party/blink/renderer/modules/accessibility/ax_object.cc
    Line 4448, Patchset 35: // happens correctly, the below line CanSetFocusAttribute() catch this case.
    Chris Harrelson . resolved
    ```suggestion
      // happens correctly, the below line CanSetFocusAttribute() catches this case.
    ```
    Aaron Leventhal

    Done

    Line 4456, Patchset 35: // TODO(accessibility) Test to see whether the following content works in
    Chris Harrelson . resolved

    Is there a plan to do that soon? A tracking bug?

    Aaron Leventhal

    Added TODO(crbug.com/350528330) as this task is listed there.

    Line 4471, Patchset 35: // TODO(accessibility) Fix WebUI and remove this.
    Chris Harrelson . resolved

    Is there a plan to do that soon? A tracking bug?

    Aaron Leventhal

    Added TODO(crbug.com/350528330) as this task is listed there.

    File third_party/blink/renderer/platform/runtime_enabled_features.json5
    Line 257, Patchset 35: name: "AccessibilityProhibitedNames",
    Chris Harrelson . resolved

    What's the goal of making this experimental instead of test?

    Aaron Leventhal

    Changed to test.

    File third_party/blink/web_tests/accessibility/aria-hidden-with-elements-expected.txt
    Line 25, Patchset 35:FAIL child_4.isIgnored should be false. Was true.
    Chris Harrelson . unresolved

    Why is this now failing?

    Aaron Leventhal

    Thanks, this should say PASS. It's now ignored because we are more correct, and we ignore the aria-hidden div that is not part of a label.

    File third_party/blink/web_tests/accessibility/computed-name-expected.txt
    Line 1, Patchset 35:CONSOLE ERROR: An accessible name was placed on a prohibited role. This causes inconsistent behavior in screen readers. For example, <div aria-label="foo"> is invalid as it is not accessible in every screen reader, because they expect only to read the inner contents of certain types of containers. Please add a valid role or put the name on a different object. As a repair technique, the browser will place the prohibited name in the accessible description field. To learn more, see the section 'Roles which cannot be named' in the ARIA specification at https://w3c.github.io/aria/#namefromprohibited.
    Chris Harrelson . unresolved

    This is hard to fix?

    Aaron Leventhal

    It's working as expected. For the element `<div role="definition" aria-label="definition name" data-knownFailure>This is a definition</div>`, we are supposed to output this error to the console. If it was in Web UI we would also trigger a DCHECK.

    So even though it says Console error, that's what we want it to say.

    Open to suggestions to make this less surprising.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Chris Harrelson
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ic59c0ca40dfedc9615f377c9a46c737557a38671
    Gerrit-Change-Number: 4035272
    Gerrit-PatchSet: 38
    Gerrit-Owner: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Reviewer: Chris Harrelson <chri...@chromium.org>
    Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alex Keng <shi...@microsoft.com>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
    Gerrit-CC: Zijie He <zij...@google.com>
    Gerrit-Attention: Chris Harrelson <chri...@chromium.org>
    Gerrit-Comment-Date: Mon, 01 Jul 2024 23:07:39 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Chris Harrelson <chri...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Aaron Leventhal (Gerrit)

    unread,
    Jul 2, 2024, 4:28:47 PM (13 hours ago) Jul 2
    to AyeAye Python Dispatcher, Chris Harrelson, Zijie He, Alex Keng, AyeAye, Tricium, Chromium LUCI CQ, chromium...@chromium.org, (Julie)Jeongeun Kim, Kevin Babbitt, Nektarios Paisios, blink-revie...@chromium.org, kinuko...@chromium.org, jmedle...@chromium.org, cros-print...@google.com, chromium-a...@chromium.org, extension...@chromium.org, print-rev...@chromium.org, filesapp...@chromium.org, cros-setti...@google.com, croissant-...@chromium.org, rginda...@chromium.org, michaelpg+wa...@chromium.org, alemat...@chromium.org, dtseng+c...@chromium.org, blundell+...@chromium.org, oshima...@chromium.org, rrsilva+wat...@google.com, shannc...@chromium.org, anastas...@google.com, katie...@chromium.org, fuchsia...@chromium.org, steimel+watch...@chromium.org, devtools-re...@chromium.org, feature-me...@chromium.org, blink-rev...@chromium.org, abigailbk...@google.com, aleventhal...@chromium.org, blink-...@chromium.org, dtseng...@chromium.org, francisjp...@google.com, hirokisa...@chromium.org, josiah...@chromium.org, kyungjunle...@google.com, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org

    Aaron Leventhal voted and added 1 comment

    Votes added by Aaron Leventhal

    Commit-Queue+1

    1 comment

    Patchset-level comments
    File-level comment, Patchset 46 (Latest):
    Aaron Leventhal . resolved

    Looks like I have more Web UI to fix first.

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ic59c0ca40dfedc9615f377c9a46c737557a38671
    Gerrit-Change-Number: 4035272
    Gerrit-PatchSet: 46
    Gerrit-Owner: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Reviewer: Chris Harrelson <chri...@chromium.org>
    Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alex Keng <shi...@microsoft.com>
    Gerrit-CC: AyeAye Python Dispatcher <android-build-ayeay...@system.gserviceaccount.com>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
    Gerrit-CC: Zijie He <zij...@google.com>
    Gerrit-Comment-Date: Tue, 02 Jul 2024 20:28:38 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages