Refactors code that deals with editable regions in Blink and adds tests [chromium/src : main]

4 views
Skip to first unread message

Dominic Mazzoni (Gerrit)

unread,
Apr 7, 2021, 6:19:07 PM4/7/21
to Nektarios Paisios, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Chromium LUCI CQ, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

Attention is currently required from: Nektarios Paisios.

View Change

1 comment:

  • Patchset:

    • Patch Set #21:

      At first glance the test failures seem like ones that should
      be rebaselined; it's making contenteditable reported as
      multiline instead of single-line, which is probably
      reasonable.

To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
Gerrit-Change-Number: 2780362
Gerrit-PatchSet: 21
Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alice Boxhall <abox...@chromium.org>
Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-Attention: Nektarios Paisios <nek...@chromium.org>
Gerrit-Comment-Date: Wed, 07 Apr 2021 22:18:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Nektarios Paisios (Gerrit)

unread,
Apr 8, 2021, 6:54:55 AM4/8/21
to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Chromium LUCI CQ, Dominic Mazzoni, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

View Change

1 comment:

  • Patchset:

    • Patch Set #23:

      @Dominic
      I went through all the test failures and documented what changed in the patch's description.

To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
Gerrit-Change-Number: 2780362
Gerrit-PatchSet: 23
Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alice Boxhall <abox...@chromium.org>
Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-Comment-Date: Thu, 08 Apr 2021 10:54:38 +0000

Dominic Mazzoni (Gerrit)

unread,
Apr 8, 2021, 6:45:24 PM4/8/21
to Nektarios Paisios, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Chromium LUCI CQ, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

Attention is currently required from: Nektarios Paisios.

Patch set 29:Code-Review +1

View Change

15 comments:

  • Patchset:

  • File chrome/browser/resources/chromeos/accessibility/chromevox/background/background_test.js:

    • Patch Set #29, Line 2444: <div role="textbox" contenteditable>

      Would it also work to add aria-multiline="true" to fix this test?
      What happened in this test without the fix?

  • File content/browser/accessibility/dump_accessibility_tree_browsertest.cc:

    • Patch Set #29, Line 1696: DISABLED_AccessibilityContenteditablePlaintextWithRole) {

      Are you planning to come back and fix this later?
      Can you add a TODO?

  • File content/test/data/accessibility/event/aria-combo-box-next-expected-uia-win.txt:

    • Patch Set #29, Line 12: AutomationFocusChanged on role=option, name=Banana

      The changes to all "event" tests for uia-win.txt are
      probably just flakiness. I'd suggest reverting them
      from this patch.

  • File third_party/blink/renderer/modules/accessibility/ax_layout_object.cc:

  • File third_party/blink/renderer/modules/accessibility/ax_node_object.cc:

    • Patch Set #29, Line 1140: const bool is_edit_box = role == ax::mojom::blink::Role::kSearchBox ||

      Where did this check go? Don't we want to assume that a search box
      is single-line?

  • File third_party/blink/renderer/modules/accessibility/ax_object.h:

    • Patch Set #29, Line 860: // Same as `IsEditable()` but returns whether the region accepts rich text

      So to clarify, this is true anywhere within the region,
      not just on the root? It seems like it's really only
      important for us to set this on the root, right?

    • Patch Set #29, Line 1162: // Returns the associated DOM node or, if an associated layout object is

      There are some cases where this is a little ambiguous.
      For something like a list marker, LayoutObject::GetNode
      returns a pseudoelement, but LayoutObject::GeneratingNode
      returns the <li> element that generated it.

  • File third_party/blink/renderer/modules/accessibility/ax_object.cc:

    • Patch Set #29, Line 3705: if (IsNativeTextField())

      Should we call this IsAtomicTextField?

    • Patch Set #29, Line 3708: if (!GetElement() || !GetElement()->IsInCanvasSubtree())

      Where did this canvas subtree code come from, why wasn't
      it needed before? A brief comment might be nice, like
      if we're in a canvas subtree, we can't check the style.

    • Patch Set #29, Line 3711: for (const Node* n = node; n; n = n->parentNode()) {

      Perhaps this should be FlatTreeTraversal::ParentNode

    • Patch Set #29, Line 3715: // Editable states do not cross document boundaries.

      Why do we need this check? I didn't think parentNode() ever
      crossed document boundaries.

    • Patch Set #29, Line 3725: if (IsDetached() || !node)

      Why did you move this code from AXLayoutObject and AXNodeObject
      to AXObject? If it only applies to an AXObject corresponding
      to a node, I thought we always put the code in AXNodeObject.

    • Patch Set #29, Line 3806: return obj->HasContentEditableAttributeSet();

      Tiny corner case, but for the canvas subtree case
      shouldn't we check for contenteditable but not
      "plaintext-only" here?

To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
Gerrit-Change-Number: 2780362
Gerrit-PatchSet: 29
Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alice Boxhall <abox...@chromium.org>
Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-Attention: Nektarios Paisios <nek...@chromium.org>
Gerrit-Comment-Date: Thu, 08 Apr 2021 22:45:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Nektarios Paisios (Gerrit)

unread,
Apr 9, 2021, 2:31:26 PM4/9/21
to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Chromium LUCI CQ, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

Attention is currently required from: Dominic Mazzoni.

View Change

14 comments:

  • File chrome/browser/resources/chromeos/accessibility/chromevox/background/background_test.js:

    • Would it also work to add aria-multiline="true" to fix this test? […]

      The bug is that output.js in ChromeVox assumes that all nodes with a role of text field and the multiline attribute are textareas.
      I could add aria-multiline=false and hide the bug, but we agreed with David that the textbox role could be removed for now, until he can address the bug, since the textbox / searchbox roles are not as common as contenteditable on its own.

  • File content/browser/accessibility/dump_accessibility_tree_browsertest.cc:

    • Are you planning to come back and fix this later? […]

      Already fixed in a followup patch that has been LGTMed.

  • File content/test/data/accessibility/event/aria-combo-box-next-expected-uia-win.txt:

    • The changes to all "event" tests for uia-win.txt are […]

      Done

  • File third_party/blink/renderer/modules/accessibility/ax_layout_object.cc:

    • Patch Set #29, Line 275: // Requires layoutObject to be present because it relies on style

      Can you explain why this comment no longer applies?

    • This was an old requirement - a few years old. Now the information is stored in the DOM node's rare data. I am not totally certain but I don't think that computing the rare data requires a layout object to be present.
      In any case, all this complexity is hidden away in the editing code, and we don't need to concern ourselves about its intricacies since we are relying on it to tell us whether a node is editable or richly editable.

    • I deleted it on purpose because after talking to Aaron Leventhal:
      1. He agreed that it would be weird for the browser to expose an editable region where the user could type in but for the accessibility tree to say that the region is not editable because of aria-hidden, there would be an inconsistency.
      2. We didn't think that the ARIA Spec mentioned that aria-hidden affects editable and I could not find any mention of it after a quick search.

      3. We explicitly disallow aria-hidden on the body element, isn't that true? So the code should have no visible effect other than removing the editable state only from the accessibility tree, causing user confusion.

  • File third_party/blink/renderer/modules/accessibility/ax_node_object.cc:

    • Where did this check go? Don't we want to assume that a search box […]

      The default is false, unless it is a textarea or a contenteditable, or unless the aria-multiline attribute is explicitly specified. So, for a search box it would be false by default without the need to check specifically. Isn't that so?

  • File third_party/blink/renderer/modules/accessibility/ax_object.h:

    • So to clarify, this is true anywhere within the region, […]

      No because a contenteditable could have a plaintext-only nested contenteditable. There is also CSS user-modify that can do the same thing. Also, we have traditionally been setting the editable and richly editable attributes on all editable and richly editable elements, so that AT won't need to walk up to the root to determine if an editable element is also richly editable. We can revisit this if you like but this replicates the existing behavior.

    • There are some cases where this is a little ambiguous. […]

      Done

  • File third_party/blink/renderer/modules/accessibility/ax_object.cc:

    • Yes but in a followup because I need to rename the methods on the browser as well and this patch would get large.

    • Where did this canvas subtree code come from, why wasn't […]

      Done

    • Patch Set #29, Line 3711: for (const Node* n = node; n; n = n->parentNode()) {

      Perhaps this should be FlatTreeTraversal::ParentNode

    • I checked and Editable style and contenteditable do not cross shadow tree boundaries. Am I misunderstanding something?

    • Why do we need this check? I didn't think parentNode() ever […]

      Sorry, my mistake. We just don't want to go above the body element and reach the HTML element, since we now include the HTML element in the accessibility tree. Just to be on the safe side. It won't cross document boundaries. I updated the comment.

    • Why did you move this code from AXLayoutObject and AXNodeObject […]

      Done

    • Tiny corner case, but for the canvas subtree case […]

      Done

To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
Gerrit-Change-Number: 2780362
Gerrit-PatchSet: 29
Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alice Boxhall <abox...@chromium.org>
Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-Attention: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-Comment-Date: Fri, 09 Apr 2021 18:31:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-MessageType: comment

Nektarios Paisios (Gerrit)

unread,
Apr 21, 2021, 2:15:36 PM4/21/21
to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Chromium LUCI CQ, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

Attention is currently required from: Dominic Mazzoni.

Patch set 38:Commit-Queue +2

View Change

    To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
    Gerrit-Change-Number: 2780362
    Gerrit-PatchSet: 38
    Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alice Boxhall <abox...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-Comment-Date: Wed, 21 Apr 2021 18:15:20 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Nektarios Paisios (Gerrit)

    unread,
    Apr 21, 2021, 3:14:48 PM4/21/21
    to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Chromium LUCI CQ, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt
    Gerrit-Comment-Date: Wed, 21 Apr 2021 19:14:32 +0000

    Nektarios Paisios (Gerrit)

    unread,
    Apr 22, 2021, 8:10:40 AM4/22/21
    to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Chromium LUCI CQ, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

    Attention is currently required from: Dominic Mazzoni.

    Patch set 39:Commit-Queue +2

    View Change

      To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
      Gerrit-Change-Number: 2780362
      Gerrit-PatchSet: 39
      Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
      Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
      Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
      Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
      Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
      Gerrit-CC: Alice Boxhall <abox...@chromium.org>
      Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
      Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
      Gerrit-Attention: Dominic Mazzoni <dmaz...@chromium.org>
      Gerrit-Comment-Date: Thu, 22 Apr 2021 12:10:25 +0000

      Chromium LUCI CQ (Gerrit)

      unread,
      Apr 22, 2021, 9:26:40 AM4/22/21
      to Nektarios Paisios, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

      Attention is currently required from: Dominic Mazzoni.

      Failed builds:
      luci.chromium.try/linux-chromeos-rel JOB_FAILED https://cr-buildbucket.appspot.com/build/8849251185007983456
      1 Test Suite(s) failed.

      **browser_tests** failed because of:

      - ChromeVoxBackgroundTest.NavigateOutOfMultiline

      View Change

        To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
        Gerrit-Change-Number: 2780362
        Gerrit-PatchSet: 39
        Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
        Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
        Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
        Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
        Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
        Gerrit-CC: Alice Boxhall <abox...@chromium.org>
        Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
        Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
        Gerrit-Attention: Dominic Mazzoni <dmaz...@chromium.org>
        Gerrit-Comment-Date: Thu, 22 Apr 2021 13:26:37 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: No
        Gerrit-MessageType: comment

        Nektarios Paisios (Gerrit)

        unread,
        Apr 22, 2021, 11:34:23 AM4/22/21
        to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Chromium LUCI CQ, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

        Attention is currently required from: Dominic Mazzoni.

        Patch set 40:Commit-Queue +2

        View Change

          To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: chromium/src
          Gerrit-Branch: main
          Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
          Gerrit-Change-Number: 2780362
          Gerrit-PatchSet: 40
          Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
          Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
          Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
          Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
          Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
          Gerrit-CC: Alice Boxhall <abox...@chromium.org>
          Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
          Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
          Gerrit-Attention: Dominic Mazzoni <dmaz...@chromium.org>
          Gerrit-Comment-Date: Thu, 22 Apr 2021 15:34:10 +0000

          Chromium LUCI CQ (Gerrit)

          unread,
          Apr 22, 2021, 12:44:43 PM4/22/21
          to Nektarios Paisios, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

          Attention is currently required from: Dominic Mazzoni.

          Failed builds:
          luci.chromium.try/linux-chromeos-rel JOB_FAILED https://cr-buildbucket.appspot.com/build/8849238359271453072

          1 Test Suite(s) failed.

          **browser_tests** failed because of:

          - ChromeVoxBackgroundTest.NavigateOutOfMultiline

          View Change

            To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

            Gerrit-Project: chromium/src
            Gerrit-Branch: main
            Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
            Gerrit-Change-Number: 2780362
            Gerrit-PatchSet: 40
            Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
            Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
            Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
            Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
            Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
            Gerrit-CC: Alice Boxhall <abox...@chromium.org>
            Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
            Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
            Gerrit-Attention: Dominic Mazzoni <dmaz...@chromium.org>
            Gerrit-Comment-Date: Thu, 22 Apr 2021 16:44:39 +0000

            Nektarios Paisios (Gerrit)

            unread,
            Apr 22, 2021, 3:47:32 PM4/22/21
            to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Chromium LUCI CQ, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

            Attention is currently required from: Dominic Mazzoni.

            Patch set 41:Commit-Queue +2

            View Change

              To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

              Gerrit-Project: chromium/src
              Gerrit-Branch: main
              Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
              Gerrit-Change-Number: 2780362
              Gerrit-PatchSet: 41
              Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
              Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
              Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
              Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
              Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
              Gerrit-CC: Alice Boxhall <abox...@chromium.org>
              Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
              Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
              Gerrit-Attention: Dominic Mazzoni <dmaz...@chromium.org>
              Gerrit-Comment-Date: Thu, 22 Apr 2021 19:47:16 +0000

              Chromium LUCI CQ (Gerrit)

              unread,
              Apr 22, 2021, 6:50:21 PM4/22/21
              to Nektarios Paisios, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

              Chromium LUCI CQ submitted this change.

              View Change

              Approvals: Dominic Mazzoni: Looks good to me Nektarios Paisios: Commit
              Refactors code that deals with editable regions in Blink and adds tests

              Methods that are used to determine if an AXObject is a text field,
              as well as other related attributes, have been moved to
              AXObject, since they only have a single override, and
              have been simplified.
              They have been extended so that they now work in canvas fallback content,
              and a set of tests have been added.

              Additionally, the kIsEditableRoot attribute is only exposed on contenteditable
              regions. In a followup patch it will be renamed to kIsContentEditableRoot.

              After this simplification / refactoring the following changes can be observed:
              1. Inline text boxes now correctly get the editable and richly editable states.
              2. List bullets now correctly drop the editable state.
              This was a request from screen reader vendors too.
              3. Inner content editables now correctly get the kIsEditableRoot attribute.
              4. The multiline state is correctly assigned to all multiline text fields
              including ARIA ones.
              5. Atomic text fields, input and textarea, do not get the editable root attribute.
              This is now implied due to their role.
              6. <input type=number> and ARIA spinners do not expose their descendants
              to platform APIs, as they are leaf nodes.
              They were inconsistently marked as leaf nodes before, i.e. they were
              marked as leaves only in certain cases.


              R=dmaz...@chromium.org, aleve...@chromium.org

              AX-Relnotes: n/a.
              Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
              Bug: 1188065
              Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2780362
              Commit-Queue: Nektarios Paisios <nek...@chromium.org>
              Reviewed-by: Dominic Mazzoni <dmaz...@chromium.org>
              Cr-Commit-Position: refs/heads/master@{#875394}
              ---
              M chrome/browser/resources/chromeos/accessibility/chromevox/background/background_test.js
              M chrome/browser/resources/chromeos/accessibility/chromevox/background/editing/editing_test.js
              M content/browser/accessibility/ax_platform_node_textrangeprovider_win_browsertest.cc
              M content/browser/accessibility/browser_accessibility_cocoa.mm
              M content/browser/accessibility/dump_accessibility_tree_browsertest.cc
              M content/test/data/accessibility/aria/aria-invalid-expected-android.txt
              M content/test/data/accessibility/aria/aria-invalid-expected-blink.txt
              M content/test/data/accessibility/aria/aria-invalid-expected-win.txt
              M content/test/data/accessibility/aria/aria-spinbutton-expected-auralinux.txt
              M content/test/data/accessibility/aria/aria-spinbutton-expected-mac.txt
              M content/test/data/accessibility/aria/aria-spinbutton-expected-win.txt
              M content/test/data/accessibility/aria/aria-textbox-with-rich-text-expected-auralinux.txt
              M content/test/data/accessibility/aria/aria-textbox-with-rich-text-expected-win.txt
              M content/test/data/accessibility/aria/aria-valuemax-expected-auralinux.txt
              M content/test/data/accessibility/aria/aria-valuemax-expected-mac.txt
              M content/test/data/accessibility/aria/aria-valuemax-expected-win.txt
              M content/test/data/accessibility/aria/aria-valuemin-expected-auralinux.txt
              M content/test/data/accessibility/aria/aria-valuemin-expected-mac.txt
              M content/test/data/accessibility/aria/aria-valuemin-expected-win.txt
              M content/test/data/accessibility/aria/input-text-aria-placeholder-expected-mac.txt
              M content/test/data/accessibility/event/text-selection-changed-expected-auralinux.txt
              M content/test/data/accessibility/html/action-verbs-expected-blink.txt
              M content/test/data/accessibility/html/action-verbs-expected-win.txt
              M content/test/data/accessibility/html/contenteditable-br-disable-ng-layout-expected-blink.txt
              M content/test/data/accessibility/html/contenteditable-br-expected-blink.txt
              M content/test/data/accessibility/html/contenteditable-descendants-expected-auralinux.txt
              M content/test/data/accessibility/html/contenteditable-descendants-expected-blink.txt
              M content/test/data/accessibility/html/contenteditable-descendants-expected-win.txt
              M content/test/data/accessibility/html/contenteditable-descendants-with-selection-expected-auralinux.txt
              M content/test/data/accessibility/html/contenteditable-descendants-with-selection-expected-blink.txt
              M content/test/data/accessibility/html/contenteditable-descendants-with-selection-expected-win.txt
              M content/test/data/accessibility/html/contenteditable-docs-li-disable-ng-layout-expected-blink.txt
              M content/test/data/accessibility/html/contenteditable-docs-li-expected-blink.txt
              M content/test/data/accessibility/html/contenteditable-spans-disable-ng-layout-expected-blink.txt
              M content/test/data/accessibility/html/contenteditable-spans-expected-blink.txt
              M content/test/data/accessibility/html/contenteditable-with-embedded-contenteditables-expected-auralinux.txt
              M content/test/data/accessibility/html/contenteditable-with-embedded-contenteditables-expected-blink.txt
              M content/test/data/accessibility/html/contenteditable-with-embedded-contenteditables-expected-mac.txt
              M content/test/data/accessibility/html/contenteditable-with-embedded-contenteditables-expected-win.txt
              M content/test/data/accessibility/html/design-mode-expected-blink.txt
              M content/test/data/accessibility/html/input-date-expected-auralinux.txt
              M content/test/data/accessibility/html/input-date-expected-mac.txt
              M content/test/data/accessibility/html/input-date-expected-uia-win.txt
              M content/test/data/accessibility/html/input-date-expected-win.txt
              M content/test/data/accessibility/html/input-date-with-popup-open-expected-auralinux.txt
              M content/test/data/accessibility/html/input-date-with-popup-open-expected-mac.txt
              M content/test/data/accessibility/html/input-date-with-popup-open-expected-uia-win.txt
              M content/test/data/accessibility/html/input-date-with-popup-open-expected-win.txt
              M content/test/data/accessibility/html/input-datetime-local-expected-auralinux.txt
              M content/test/data/accessibility/html/input-datetime-local-expected-uia-win.txt
              M content/test/data/accessibility/html/input-datetime-local-expected-win.txt
              M content/test/data/accessibility/html/input-month-expected-auralinux.txt
              M content/test/data/accessibility/html/input-month-expected-mac.txt
              M content/test/data/accessibility/html/input-number-expected-android.txt
              M content/test/data/accessibility/html/input-number-expected-auralinux.txt
              M content/test/data/accessibility/html/input-number-expected-mac.txt
              M content/test/data/accessibility/html/input-number-expected-win.txt
              M content/test/data/accessibility/html/input-text-expected-mac.txt
              M content/test/data/accessibility/html/input-text-read-only-expected-mac.txt
              M content/test/data/accessibility/html/input-time-expected-auralinux.txt
              M content/test/data/accessibility/html/input-time-expected-uia-win.txt
              M content/test/data/accessibility/html/input-time-expected-win.txt
              M content/test/data/accessibility/html/input-time-with-popup-open-expected-auralinux.txt
              M content/test/data/accessibility/html/input-time-with-popup-open-expected-mac.txt
              M content/test/data/accessibility/html/input-time-with-popup-open-expected-uia-win.txt
              M content/test/data/accessibility/html/input-time-with-popup-open-expected-win.txt
              M content/test/data/accessibility/html/input-types-expected-android.txt
              M content/test/data/accessibility/html/input-types-expected-auralinux.txt
              M content/test/data/accessibility/html/input-types-expected-blink.txt
              M content/test/data/accessibility/html/input-types-expected-mac.txt
              M content/test/data/accessibility/html/input-types-expected-win.txt
              M content/test/data/accessibility/html/input-types-with-placeholder-expected-blink.txt
              M content/test/data/accessibility/html/input-types-with-value-and-placeholder-expected-blink.txt
              M content/test/data/accessibility/html/input-types-with-value-expected-blink.txt
              M content/test/data/accessibility/html/input-week-expected-auralinux.txt
              M content/test/data/accessibility/html/input-week-expected-mac.txt
              M content/test/data/accessibility/html/input-week-expected-uia-win.txt
              M content/test/data/accessibility/html/input-week-expected-win.txt
              M content/test/data/accessibility/html/node-changed-crash-in-editable-text-expected-win.txt
              M content/test/data/accessibility/html/output-expected-android.txt
              M content/test/data/accessibility/html/output-expected-mac.txt
              M content/test/data/accessibility/html/ul-contenteditable-expected-blink.txt
              M content/test/data/accessibility/html/ul-contenteditable-expected-win.txt
              M content/test/data/accessibility/scripts/set-selection-textarea-expected-mac.txt
              M third_party/blink/renderer/modules/accessibility/ax_layout_object.cc
              M third_party/blink/renderer/modules/accessibility/ax_layout_object.h
              M third_party/blink/renderer/modules/accessibility/ax_layout_object_test.cc
              M third_party/blink/renderer/modules/accessibility/ax_node_object.cc
              M third_party/blink/renderer/modules/accessibility/ax_node_object.h
              M third_party/blink/renderer/modules/accessibility/ax_object.cc
              M third_party/blink/renderer/modules/accessibility/ax_object.h
              M third_party/blink/renderer/modules/accessibility/ax_object_test.cc
              M third_party/blink/renderer/modules/accessibility/ax_relation_cache.cc
              D third_party/blink/web_tests/accessibility/contenteditable-on-aria-hidden-body.html
              M third_party/blink/web_tests/accessibility/editable-root.html
              M ui/accessibility/ax_node.cc
              96 files changed, 682 insertions(+), 614 deletions(-)


              29 is the latest approved patch-set. The change was submitted with unreviewed changes in the following files: The name of the file: chrome/browser/resources/chromeos/accessibility/chromevox/background/background_test.js Insertions: 32, Deletions: 32. ``` @@ -2437:2441, +2437:2442 @@ - TEST_F('ChromeVoxBackgroundTest', 'NavigateOutOfMultiline', function() { - const mockFeedback = this.createMockFeedback(); - this.runWithLoadedTree( - ` + TEST_F( + 'ChromeVoxBackgroundTest', 'DISABLED_NavigateOutOfMultiline', function() { + const mockFeedback = this.createMockFeedback(); + this.runWithLoadedTree( + ` @@ -2444:2446, +2445:2446 @@ - Testing testing<br> - one two three + Testing testing<br>one two three @@ -2449:2474, +2449:2473 @@ - function(root) { - const textField = root.find({role: RoleType.TEXT_FIELD}); - mockFeedback.call(textField.focus.bind(textField)) - .expectSpeech('Testing testing\none two three') - .expectSpeech('Edit text') - .call(doCmd('nextLine')) - .expectSpeech('one two three') - .call(doCmd('nextLine')) - .expectSpeech('after') - - // In reverse (explicitly focus, instead of moving to previous line, - // because all subsequent commands require the text field be focused - // first): - .clearPendingOutput() - .call(textField.focus.bind(textField)) - .expectSpeech('Edit text') - .call(doCmd('nextLine')) - .expectSpeech('one two three') - .call(doCmd('previousLine')) - .expectSpeech('Testing testing') - .call(doCmd('previousLine')) - .expectSpeech('before') - .replay(); - }); - }); + function(root) { + const contentEditable = + root.find({attributes: {editableRoot: true}}); + mockFeedback.call(contentEditable.focus.bind(contentEditable)) + .expectSpeech(/Testing testing\s+one two three/) + .call(doCmd('nextLine')) + .expectSpeech('one two three') + .call(doCmd('nextLine')) + .expectSpeech('after') + + // In reverse (explicitly focus, instead of moving to previous + // line, because all subsequent commands require the content + // editable to be focused first): + .clearPendingOutput() + .call(contentEditable.focus.bind(contentEditable)) + .call(doCmd('nextLine')) + .expectSpeech('one two three') + .call(doCmd('previousLine')) + .expectSpeech('Testing testing') + .call(doCmd('previousLine')) + .expectSpeech('before') + .replay(); + }); + }); @@ -2517:2518, +2516:2517 @@ - {annotation: [new Output.Action()]}); + {annotation: [new OutputAction()]}); @@ -2984:2987, +2983:2984 @@ - // This was overridden in setUp() for most all tests, but we want the - // production behavior here. - Output.ROLE_INFO_[RoleType.DIALOG]['outputContextFirst'] = true; + this.resetContextualOutput(); @@ +2994:2995 @@ + <p>end</p> @@ -3003:3005, +3001:3017 @@ - .expectSpeech('Welcome') - .expectSpeech('Heading 1') + .expectSpeech('Welcome', 'Heading 1') + .call(doCmd('nextObject')) + .expectSpeech('This is some introductory text') + .call(doCmd('nextObject')) + .expectSpeech('Exit', 'Button') + .call(doCmd('nextObject')) + .expectSpeech(`Let's go`, 'Button') + .call(doCmd('nextObject')) + .expectSpeech('end') + + .call(doCmd('previousObject')) + .expectSpeech(`Let's go`, 'Button') + .expectSpeech('Setup', 'Dialog') + .expectSpeech( + `Welcome This is some introductory text Exit Let's go`) + @@ -3051:3052, +3063:3064 @@ - this.runWithLoadedTree(``, function() { + this.runWithLoadedTree('<p>test</p>', function() { @@ -3065:3066, +3077:3078 @@ - TEST_F('ChromeVoxBackgroundTest', 'WrapTextFieldAtEndOfDoc', function() { + TEST_F('ChromeVoxBackgroundTest', 'WrapContentEditableAtEndOfDoc', function() { @@ -3067:3068, +3079:3081 @@ - const site = `<p>start</p><div role="textbox" contenteditable></div>`; + const site = `<p>start</p> + <div role="textbox" contenteditable aria-multiline="false"></div>`; ``` The name of the file: chrome/browser/resources/chromeos/accessibility/chromevox/background/editing/editing_test.js Insertions: 1, Deletions: 2. ``` @@ -1197:1198, +1197:1198 @@ - const input = root.find({role: RoleType.TEXT_FIELD}); + const input = root.find({attributes: {editableRoot: true}}); @@ -1211:1212 @@ - .expectSpeech('this , deleted') ``` The name of the file: content/browser/accessibility/ax_platform_node_textrangeprovider_win_browsertest.cc Insertions: 3, Deletions: 3. ``` @@ -492:494, +492:494 @@ - GetAttributeValueIsReadonlyRichTextField) { - LoadInitialAccessibilityTreeFromHtml(std::string(R"HTML( + DISABLED_GetAttributeValueIsReadonlyRichTextField) { + LoadInitialAccessibilityTreeFromHtml(R"HTML( @@ -508:509, +508:509 @@ - )HTML")); + )HTML"); @@ -2659:2660, +2659:2660 @@ - EXPECT_TRUE(ExecuteScript( + EXPECT_TRUE(ExecJs( @@ -2687:2688, +2687:2688 @@ - EXPECT_TRUE(ExecuteScript( + EXPECT_TRUE(ExecJs( @@ -2719:2720, +2719:2720 @@ - EXPECT_TRUE(ExecuteScript( + EXPECT_TRUE(ExecJs( @@ -2757:2758, +2757:2758 @@ - EXPECT_TRUE(ExecuteScript( + EXPECT_TRUE(ExecJs( @@ -2794:2795, +2794:2795 @@ - EXPECT_TRUE(ExecuteScript( + EXPECT_TRUE(ExecJs( ``` The name of the file: content/test/data/accessibility/event/aria-combo-box-next-expected-uia-win.txt Insertions: 2, Deletions: 1. ``` @@ +11:12 @@ + AutomationFocusChanged on role=option, name=Banana @@ -13:14, +14:16 @@ - SelectionItem_ElementSelected on role=option, name=Banana + SelectionItem_ElementSelected on role=option, name=Banana + ``` The name of the file: content/test/data/accessibility/event/menulist-collapse-next-expected-uia-win.txt Insertions: 1, Deletions: 2. ``` @@ -2:3 @@ - AutomationFocusChanged on role=combobox @@ -10:11, +9:11 @@ - ValueValue changed on role=combobox + ValueValue changed on role=combobox + ``` The name of the file: third_party/blink/renderer/modules/accessibility/ax_layout_object.cc Insertions: 5, Deletions: 0. ``` @@ +96:97 @@ + #include "ui/accessibility/ax_role_properties.h" @@ +115:120 @@ + void AXLayoutObject::Trace(Visitor* visitor) const { + visitor->Trace(layout_object_); + AXNodeObject::Trace(visitor); + } + @@ -134:135, +140:141 @@ - auto* box = To<LayoutBox>(layout_object_); + auto* box = To<LayoutBox>(layout_object_.Get()); @@ +173:193 @@ + static bool ShouldIgnoreListItem(Node* node) { + DCHECK(node); + + // http://www.w3.org/TR/wai-aria/complete#presentation + // A list item is presentational if its parent is a native list but + // it has an explicit ARIA role set on it that's anything other than "list". + Element* parent = FlatTreeTraversal::ParentElement(*node); + if (!parent) + return false; + + if (IsA<HTMLMenuElement>(*parent) || IsA<HTMLUListElement>(*parent) || + IsA<HTMLOListElement>(*parent)) { + AtomicString role = AccessibleNode::GetPropertyOrARIAAttribute( + parent, AOMStringProperty::kRole); + if (!role.IsEmpty() && role != "list") + return true; + } + return false; + } + @@ -172:173, +198:201 @@ - if (layout_object_->IsListItemIncludingNG() || IsA<HTMLLIElement>(node)) + if (IsA<HTMLLIElement>(node)) { + if (ShouldIgnoreListItem(node)) + return ax::mojom::blink::Role::kNone; @@ -174:175, +202:208 @@ - if (layout_object_->IsListMarkerIncludingAll()) + } + + if (layout_object_->IsListMarkerIncludingAll()) { + Node* list_item = layout_object_->GeneratingNode(); + if (list_item && ShouldIgnoreListItem(list_item)) + return ax::mojom::blink::Role::kNone; @@ +209:213 @@ + } + + if (layout_object_->IsListItemIncludingNG()) + return ax::mojom::blink::Role::kListItem; @@ -203:204, +240:241 @@ - if (IsA<LayoutView>(layout_object_)) + if (IsA<LayoutView>(*layout_object_)) @@ +519:532 @@ + // Ignore text inside of an ignored <label>. + // To save processing, only walk up the ignored objects. + // This means that other interesting objects inside the <label> will + // cause the text to be unignored. + AXObject* ancestor = ParentObject(); + while (ancestor && ancestor->AccessibilityIsIgnored()) { + if (ancestor->RoleValue() == ax::mojom::blink::Role::kLabelText) { + if (ignored_reasons) + ignored_reasons->push_back(IgnoredReason(kAXPresentational)); + return true; + } + ancestor = ancestor->ParentObject(); + } @@ -953:954, +1003:1004 @@ - auto* layout_text = To<LayoutText>(layout_object_); + auto* layout_text = To<LayoutText>(layout_object_.Get()); @@ -974:975, +1024:1025 @@ - To<LayoutListMarker>(layout_object_)->TextAlternative(); + To<LayoutListMarker>(layout_object_.Get())->TextAlternative(); @@ -1014:1015, +1064:1065 @@ - PaintLayer* layer = To<LayoutBox>(layout_object_)->Layer(); + PaintLayer* layer = To<LayoutBox>(layout_object_.Get())->Layer(); @@ -1056:1057, +1106:1108 @@ - if (result && result->AccessibilityIsIgnored()) { + + while (result && result->AccessibilityIsIgnored()) { @@ -1058:1064, +1109:1115 @@ - // control. - if (auto* ax_object = DynamicTo<AXLayoutObject>(result)) { - AXObject* control_object = - ax_object->CorrespondingControlAXObjectForLabelElement(); - if (control_object && control_object->NameFromLabelElement()) - return control_object; + // control. The label is ignored because it's already reflected in the name. + if (auto* label = DynamicTo<HTMLLabelElement>(result->GetNode())) { + if (HTMLElement* control = label->control()) { + if (AXObject* ax_control = AXObjectCache().GetOrCreate(control)) + return ax_control; + } @@ -1066:1067, +1117:1118 @@ - result = result->ParentObjectUnignored(); + result = result->ParentObject(); ``` The name of the file: third_party/blink/renderer/modules/accessibility/ax_node_object.cc Insertions: 41, Deletions: 0. ``` @@ +324:326 @@ + DCHECK(GetDocument()); + @@ -333:343, +335:338 @@ - if (HasInheritedPresentationalRole()) { - if (ignored_reasons) { - const AXObject* inherits_from = InheritsPresentationalRoleFrom(); - if (inherits_from == this) { - ignored_reasons->push_back(IgnoredReason(kAXPresentational)); - } else { - ignored_reasons->push_back( - IgnoredReason(kAXInheritsPresentation, inherits_from)); - } - } + if (IsPresentational()) { + if (ignored_reasons) + ignored_reasons->push_back(IgnoredReason(kAXPresentational)); @@ -350:352, +345:346 @@ - if (GetDocument() && GetDocument()->GetPage() && - GetDocument()->GetPage()->InsidePortal()) { + if (GetDocument()->GetPage() && GetDocument()->GetPage()->InsidePortal()) @@ +347:355 @@ + + Node* node = GetNode(); + if (!node) { + // Nodeless pseudo element images are included, even if they don't have CSS + // alt text. This can allow auto alt to be applied to them. + if (IsImage()) + return kIncludeObject; + return kDefaultBehavior; @@ -358:381, +360:362 @@ - // Ignore labels that are already referenced by a control but are not set to - // be focusable. - AXObject* control_ax_object = CorrespondingControlAXObjectForLabelElement(); - if (control_ax_object && control_ax_object->IsCheckboxOrRadio() && - control_ax_object->NameFromLabelElement() && - AccessibleNode::GetPropertyOrARIAAttribute( - LabelElementContainer(), AOMStringProperty::kRole) == g_null_atom) { - AXObject* label_ax_object = CorrespondingLabelAXObject(); - // If the label is set to be focusable, we should expose it. - if (label_ax_object && label_ax_object->CanSetFocusAttribute()) - return kIncludeObject; - - if (ignored_reasons) { - if (label_ax_object && label_ax_object != this) - ignored_reasons->push_back( - IgnoredReason(kAXLabelContainer, label_ax_object)); - - ignored_reasons->push_back(IgnoredReason(kAXLabelFor, control_ax_object)); - } - return kIgnoreObject; - } - - if (GetNode() && !IsA<HTMLBodyElement>(GetNode()) && CanSetFocusAttribute()) + // All focusable elements except the <body> are included. + if (!IsA<HTMLBodyElement>(node) && CanSetFocusAttribute()) @@ -398:400, +379:381 @@ - if (GetNode() && (GetNode()->HasTagName(html_names::kHeaderTag) || - GetNode()->HasTagName(html_names::kFooterTag))) + if (node->HasTagName(html_names::kHeaderTag) || + node->HasTagName(html_names::kFooterTag)) @@ -417:422 @@ - // Don't ignore labels, because they serve as TitleUIElements. - Node* node = GetNode(); - if (IsA<HTMLLabelElement>(node)) - return kIncludeObject; - @@ -473:485 @@ - // If this element has aria attributes on it, it should not be ignored. - if (HasGlobalARIAAttribute()) - return kIncludeObject; - - bool has_non_empty_alt_attribute = !GetAttribute(kAltAttr).IsEmpty(); - if (IsImage()) { - if (has_non_empty_alt_attribute || GetAttribute(kAltAttr).IsNull()) - return kIncludeObject; - else if (ignored_reasons) - ignored_reasons->push_back(IgnoredReason(kAXEmptyAlt)); - return kIgnoreObject; - } @@ -489:495, +453:454 @@ - // - // These checks are simplified in the interest of execution speed; - // for example, any element having an alt attribute will make it - // not ignored, rather than just images. - if (HasAriaAttribute() || !GetAttribute(kTitleAttr).IsEmpty() || - has_non_empty_alt_attribute) + if (HasAriaAttribute() || !GetAttribute(kTitleAttr).IsEmpty()) @@ +456:468 @@ + if (IsImage()) { + String alt = GetAttribute(kAltAttr); + // A null alt attribute means the attribute is not present. We assume this + // is a mistake, and expose the image so that it can be repaired. + // In contrast, alt="" is treated as intentional markup to ignore the image. + if (!alt.IsEmpty() || alt.IsNull()) + return kIncludeObject; + if (ignored_reasons) + ignored_reasons->push_back(IgnoredReason(kAXEmptyAlt)); + return kIgnoreObject; + } + @@ -500:501, +471:472 @@ - if (node && IsA<HTMLSpanElement>(node)) { + if (IsA<HTMLSpanElement>(node)) { @@ +477:490 @@ + // Ignore labels that are already used to name a control. + // See IsRedundantLabel() for more commentary. + if (HTMLLabelElement* label = DynamicTo<HTMLLabelElement>(node)) { + if (IsRedundantLabel(label)) { + if (ignored_reasons) { + ignored_reasons->push_back( + IgnoredReason(kAXLabelFor, AXObjectCache().Get(label->control()))); + } + return kIgnoreObject; + } + return kIncludeObject; + } + @@ -594:657 @@ - static bool IsListElement(Node* node) { - return IsA<HTMLUListElement>(*node) || IsA<HTMLOListElement>(*node) || - IsA<HTMLDListElement>(*node); - } - - static bool IsRequiredOwnedElement(AXObject* parent, - ax::mojom::blink::Role current_role, - HTMLElement* current_element) { - Node* parent_node = parent->GetNode(); - auto* parent_html_element = DynamicTo<HTMLElement>(parent_node); - if (!parent_html_element) - return false; - - if (current_role == ax::mojom::blink::Role::kListItem) - return IsListElement(parent_node); - if (current_role == ax::mojom::blink::Role::kListMarker) - return IsA<HTMLLIElement>(*parent_node); - - if (!current_element) - return false; - if (IsA<HTMLTableCellElement>(*current_element)) - return IsA<HTMLTableRowElement>(*parent_node); - if (IsA<HTMLTableRowElement>(*current_element)) - return IsA<HTMLTableSectionElement>(parent_html_element); - - // In case of ListboxRole and its child, ListBoxOptionRole, inheritance of - // presentation role is handled in AXListBoxOption because ListBoxOption Role - // doesn't have any child. - // If it's just ignored because of presentation, we can't see any AX tree - // related to ListBoxOption. - return false; - } - - const AXObject* AXNodeObject::InheritsPresentationalRoleFrom() const { - // ARIA states if an item can get focus, it should not be presentational. - if (CanSetFocusAttribute()) - return nullptr; - - if (IsPresentational()) - return this; - - // http://www.w3.org/TR/wai-aria/complete#presentation - // ARIA spec says that the user agent MUST apply an inherited role of - // presentation to any owned elements that do not have an explicit role. - if (AriaRoleAttribute() != ax::mojom::blink::Role::kUnknown) - return nullptr; - - AXObject* parent = ParentObject(); - if (!parent) - return nullptr; - - auto* element = DynamicTo<HTMLElement>(GetNode()); - if (!parent->HasInheritedPresentationalRole()) - return nullptr; - - // ARIA spec says that when a parent object is presentational and this object - // is a required owned element of that parent, then this object is also - // presentational. - if (IsRequiredOwnedElement(parent, RoleValue(), element)) - return parent; - return nullptr; - } - @@ -1160:1161, +1081:1082 @@ - Node* node = this->GetNode(); + Node* node = GetNode(); @@ -1212:1213, +1133:1134 @@ - Node* node = this->GetNode(); + Node* node = GetNode(); @@ -1222:1223, +1143:1144 @@ - Node* node = this->GetNode(); + Node* node = GetNode(); @@ -1256:1257, +1177:1178 @@ - if (Node* node = this->GetNode()) + if (Node* node = GetNode()) @@ -1326:1327, +1247:1248 @@ - Node* node = this->GetNode(); + Node* node = GetNode(); @@ -1624:1625, +1545:1546 @@ - Node* node = this->GetNode(); + Node* node = GetNode(); @@ -1630:1631, +1551:1552 @@ - Node* node = this->GetNode(); + Node* node = GetNode(); @@ -2676:2695 @@ - bool AXNodeObject::HasAriaAttribute() const { - Element* element = GetElement(); - if (!element) - return false; - - // Explicit ARIA role should be considered an aria attribute. - if (AriaRoleAttribute() != ax::mojom::blink::Role::kUnknown) - return true; - - AttributeCollection attributes = element->AttributesWithoutUpdate(); - for (const Attribute& attr : attributes) { - // Attributes cache their uppercase names. - if (attr.GetName().LocalNameUpper().StartsWith("ARIA-")) - return true; - } - - return false; - } - @@ -2735:2736, +2637:2639 @@ - TokenVectorFromAttribute(str_dropeffects, html_names::kAriaDropeffectAttr); + TokenVectorFromAttribute(GetElement(), str_dropeffects, + html_names::kAriaDropeffectAttr); @@ +2691:2732 @@ + bool AXNodeObject::IsEditableRoot() const { + const Node* node = GetNode(); + if (IsDetached() || !node) + return false; + #if DCHECK_IS_ON() // Required in order to get Lifecycle().ToString() + DCHECK(GetDocument()); + DCHECK_GE(GetDocument()->Lifecycle().GetState(), + DocumentLifecycle::kStyleClean) + << "Unclean document style at lifecycle state " + << GetDocument()->Lifecycle().ToString(); + #endif // DCHECK_IS_ON() + + // The DOM inside native text fields is an implementation detail that should + // not be exposed to platform accessibility APIs. + if (EnclosingTextControl(node)) + return false; + + if (IsRootEditableElement(*node)) + return true; + + // Catches the case where a contenteditable is inside another contenteditable. + // This is especially important when the two nested contenteditables have + // different attributes, e.g. "true" vs. "plaintext-only". + if (HasContentEditableAttributeSet()) + return true; + + return false; + } + + bool AXNodeObject::HasContentEditableAttributeSet() const { + if (IsDetached() || !GetNode()) + return false; + + const auto* html_element = DynamicTo<HTMLElement>(GetNode()); + if (!html_element) + return false; + + String normalized_value = html_element->contentEditable(); + return normalized_value == "true" || normalized_value == "plaintext-only"; + } + @@ -3141:3146, +3085:3091 @@ - bool AXNodeObject::NameFromLabelElement() const { - // This unfortunately duplicates a bit of logic from textAlternative and - // nativeTextAlternative, but it's necessary because nameFromLabelElement - // needs to be called from computeAccessibilityIsIgnored, which isn't allowed - // to call axObjectCache->getOrCreate. + // static + bool AXNodeObject::IsNameFromLabelElement(HTMLElement* control) { + // This duplicates some logic from TextAlternative()/NativeTextAlternative(), + // but is necessary because IsNameFromLabelElement() needs to be called from + // ComputeAccessibilityIsIgnored(), which isn't allowed to call + // AXObjectCache().GetOrCreate() in random places in the tree. @@ -3147:3148, +3092:3093 @@ - if (!GetNode() && !GetLayoutObject()) + if (!control) @@ -3150:3154, +3095:3096 @@ - // Step 2A from: http://www.w3.org/TR/accname-aam-1.1 - if (IsHiddenForTextAlternativeCalculation()) - return false; - + // aria-labelledby takes precedence over <label>. @@ -3155:3161 @@ - // Try both spellings, but prefer aria-labelledby, which is the official spec. - const QualifiedName& attr = - HasAttribute(html_names::kAriaLabeledbyAttr) && - !HasAttribute(html_names::kAriaLabelledbyAttr) - ? html_names::kAriaLabeledbyAttr - : html_names::kAriaLabelledbyAttr; @@ -3163:3165, +3099:3100 @@ - ElementsFromAttribute(elements_from_attribute, attr, ids); - if (elements_from_attribute.size() > 0) + if (AriaLabelledbyElementVector(control, elements_from_attribute, ids)) @@ +3102:3103 @@ + // aria-label takes precedence over <label> . @@ -3168:3170, +3104:3106 @@ - const AtomicString& aria_label = - GetAOMPropertyOrARIAAttribute(AOMStringProperty::kLabel); + const AtomicString& aria_label = AccessibleNode::GetPropertyOrARIAAttribute( + control, AOMStringProperty::kLabel); @@ -3173:3175, +3109:3111 @@ - // Based on - // http://rawgit.com/w3c/aria/master/html-aam/html-aam.html#accessible-name-and-description-calculation + // <label> will be used. It contains the control or points via <label for>. + // Based on https://www.w3.org/TR/html-aam-1.0 @@ -3176:3181, +3112:3115 @@ - auto* html_element = DynamicTo<HTMLElement>(GetNode()); - if (html_element && html_element->IsLabelable()) { - if (html_element->labels() && html_element->labels()->length() > 0) - return true; - } + auto* labels = control->labels(); + return labels && labels->length(); + } @@ -3182:3183, +3116:3154 @@ - return false; + // static + bool AXNodeObject::IsRedundantLabel(HTMLLabelElement* label) { + // Determine if label is redundant: + // - Labelling a checkbox or radio. + // - Text was already used to name the checkbox/radio. + // - No interesting content in the label (focusable or semantically useful) + // TODO(accessibility) Consider moving this logic to the browser side. + // TODO(accessibility) Consider this for more controls, such as textboxes. + // There isn't a clear history why this is only done for checkboxes, and not + // other controls such as textboxes. It may be because the checkbox/radio + // itself is small, so this makes a nicely sized combined click target. Most + // ATs do not already have features to combine labels and controls, e.g. + // removing redundant announcements caused by having text and named controls + // as separate objects. + HTMLInputElement* input = DynamicTo<HTMLInputElement>(label->control()); + if (!input) + return false; + + if (!input->IsCheckable()) + return false; + + if (!IsNameFromLabelElement(input)) + return false; + + DCHECK_NE(input->labels()->length(), 0U); + + // Look for any first child element that is not the input control itself. + // This could be important semantically. + Element* first_child = ElementTraversal::FirstChild(*label); + if (!first_child) + return true; // No element children. + + if (first_child != input) + return false; // Has an element child that is not the input control. + + // The first child was the input control. + // If there's another child, then it won't be the input control. + return ElementTraversal::NextSibling(*first_child) == nullptr; @@ -4003:4004, +3974:3975 @@ - Node* node = this->GetNode(); + Node* node = GetNode(); @@ -4019:4020, +3990:3991 @@ - Node* node = this->GetNode(); + Node* node = GetNode(); @@ -4124:4166 @@ - AXObject* AXNodeObject::CorrespondingControlAXObjectForLabelElement() const { - HTMLLabelElement* label_element = LabelElementContainer(); - if (!label_element) - return nullptr; - - HTMLElement* corresponding_control = label_element->control(); - if (!corresponding_control) - return nullptr; - - // Make sure the corresponding control isn't a descendant of this label - // that's in the middle of being destroyed. - if (corresponding_control->GetLayoutObject() && - !corresponding_control->GetLayoutObject()->Parent()) - return nullptr; - - return AXObjectCache().GetOrCreate(corresponding_control); - } - - AXObject* AXNodeObject::CorrespondingLabelAXObject() const { - HTMLLabelElement* label_element = LabelElementContainer(); - if (!label_element) - return nullptr; - - return AXObjectCache().GetOrCreate(label_element); - } - - HTMLLabelElement* AXNodeObject::LabelElementContainer() const { - if (!GetNode()) - return nullptr; - - // the control element should not be considered part of the label - if (IsControl()) - return nullptr; - - // the link element should not be considered part of the label - if (IsLink()) - return nullptr; - - // find if this has a ancestor that is a label - return Traversal<HTMLLabelElement>::FirstAncestorOrSelf(*GetNode()); - } - @@ -4369:4370, +4298:4299 @@ - AXObject* container_parent = this->ParentObject(); + AXObject* container_parent = ParentObject(); @@ -4931:4932, +4860:4861 @@ - Document* document = this->GetDocument(); + Document* document = GetDocument(); @@ -5076:5079, +5005:5007 @@ - ElementsFromAttribute(elements_from_attribute, - html_names::kAriaDescribedbyAttr, ids); - if (!elements_from_attribute.IsEmpty()) { + if (ElementsFromAttribute(element, elements_from_attribute, + html_names::kAriaDescribedbyAttr, ids)) { @@ -5093:5094, +5021:5022 @@ - TokenVectorFromAttribute(ids, html_names::kAriaDescribedbyAttr); + TokenVectorFromAttribute(element, ids, html_names::kAriaDescribedbyAttr); @@ -5116:5117, +5044:5045 @@ - description_from = ax::mojom::blink::DescriptionFrom::kAttribute; + description_from = ax::mojom::blink::DescriptionFrom::kAriaDescription; @@ -5128:5129, +5056:5057 @@ - // value, 5.2.2 from: http://rawgit.com/w3c/aria/master/html-aam/html-aam.html + // value, 5.2.2 from: https://www.w3.org/TR/html-aam-1.0/ @@ -5131:5132, +5059:5060 @@ - description_from = ax::mojom::blink::DescriptionFrom::kAttribute; + description_from = ax::mojom::blink::DescriptionFrom::kButtonLabel; @@ -5151:5152, +5079:5080 @@ - description_from = ax::mojom::blink::DescriptionFrom::kRelatedElement; + description_from = ax::mojom::blink::DescriptionFrom::kRubyAnnotation; @@ -5187:5189, +5115:5116 @@ - // table caption, 5.9.2 from: - // http://rawgit.com/w3c/aria/master/html-aam/html-aam.html + // table caption, 5.9.2 from: https://www.w3.org/TR/html-aam-1.0/ @@ -5191:5192, +5118:5119 @@ - description_from = ax::mojom::blink::DescriptionFrom::kRelatedElement; + description_from = ax::mojom::blink::DescriptionFrom::kTableCaption; @@ -5223:5225, +5150:5151 @@ - // summary, 5.6.2 from: - // http://rawgit.com/w3c/aria/master/html-aam/html-aam.html + // summary, 5.8.2 from: https://www.w3.org/TR/html-aam-1.0/ @@ -5227:5228, +5153:5154 @@ - description_from = ax::mojom::blink::DescriptionFrom::kContents; + description_from = ax::mojom::blink::DescriptionFrom::kSummary; @@ -5246:5248, +5172:5173 @@ - // title attribute, from: - // http://rawgit.com/w3c/aria/master/html-aam/html-aam.html + // title attribute, from: https://www.w3.org/TR/html-aam-1.0/ @@ -5267:5268, +5192:5193 @@ - description_from = ax::mojom::blink::DescriptionFrom::kUninitialized; + description_from = ax::mojom::blink::DescriptionFrom::kNone; @@ +5195:5200 @@ + DCHECK(description_sources) + << "Should only reach here if description_sources are tracked"; + // Use the first non-null description. + // TODO(accessibility) Why do we need to check superceded if that will + // always be the first one? ``` The name of the file: third_party/blink/renderer/modules/accessibility/ax_node_object.h Insertions: 5, Deletions: 0. ``` @@ +40:41 @@ + class HTMLElement; @@ -65:66 @@ - const AXObject* InheritsPresentationalRoleFrom() const override; @@ -80:83, +80:81 @@ - AXObject* CorrespondingControlAXObjectForLabelElement() const; - AXObject* CorrespondingLabelAXObject() const; - HTMLLabelElement* LabelElementContainer() const; + HTMLElement* CorrespondingControlForLabelElement() const; @@ -179:180 @@ - bool HasAriaAttribute() const override; @@ +184:186 @@ + bool IsEditableRoot() const override; + bool HasContentEditableAttributeSet() const override; @@ -209:210 @@ - bool NameFromLabelElement() const override; @@ +314:317 @@ + static bool IsNameFromLabelElement(HTMLElement* control); + static bool IsRedundantLabel(HTMLLabelElement* label); + ``` The name of the file: third_party/blink/renderer/modules/accessibility/ax_object.cc Insertions: 5, Deletions: 70. ``` @@ +31:32 @@ + #include <ostream> @@ -128:130 @@ - case kAXInheritsPresentation: - return "inheritsPresentation"; @@ +231:234 @@ + constexpr wtf_size_t kNumRoles = + static_cast<wtf_size_t>(ax::mojom::blink::Role::kMaxValue) + 1; + @@ -239:241, +241:243 @@ - const char* aria_role; - ax::mojom::blink::Role webcore_role; + const char* role_name; + ax::mojom::blink::Role role; @@ -244:245, +246:251 @@ - const RoleEntry kRoles[] = { + // This is used for the following: + // 1. Map from an ARIA role to the internal role when building tree. + // 2. Map from an internal role to an ARIA role name, for debugging, the + // xml-roles object attribute and element.computedRole. + const RoleEntry kAriaRoles[] = { @@ -376:377 @@ - {"text", ax::mojom::blink::Role::kStaticText}, @@ -386:596, +391:394 @@ - struct InternalRoleEntry { - ax::mojom::blink::Role webcore_role; - const char* internal_role_name; - }; - - const InternalRoleEntry kInternalRoles[] = { - {ax::mojom::blink::Role::kNone, "None"}, - {ax::mojom::blink::Role::kAbbr, "Abbr"}, - {ax::mojom::blink::Role::kAlertDialog, "AlertDialog"}, - {ax::mojom::blink::Role::kAlert, "Alert"}, - {ax::mojom::blink::Role::kAnchor, "Anchor"}, - {ax::mojom::blink::Role::kComment, "Comment"}, - {ax::mojom::blink::Role::kApplication, "Application"}, - {ax::mojom::blink::Role::kArticle, "Article"}, - {ax::mojom::blink::Role::kAudio, "Audio"}, - {ax::mojom::blink::Role::kBanner, "Banner"}, - {ax::mojom::blink::Role::kBlockquote, "Blockquote"}, - {ax::mojom::blink::Role::kButton, "Button"}, - {ax::mojom::blink::Role::kCanvas, "Canvas"}, - {ax::mojom::blink::Role::kCaption, "Caption"}, - {ax::mojom::blink::Role::kCaret, "Caret"}, - {ax::mojom::blink::Role::kCell, "Cell"}, - {ax::mojom::blink::Role::kCheckBox, "CheckBox"}, - {ax::mojom::blink::Role::kClient, "Client"}, - {ax::mojom::blink::Role::kCode, "Code"}, - {ax::mojom::blink::Role::kColorWell, "ColorWell"}, - {ax::mojom::blink::Role::kColumnHeader, "ColumnHeader"}, - {ax::mojom::blink::Role::kColumn, "Column"}, - {ax::mojom::blink::Role::kComboBoxGrouping, "ComboBox"}, - {ax::mojom::blink::Role::kComboBoxMenuButton, "ComboBox"}, - {ax::mojom::blink::Role::kComplementary, "Complementary"}, - {ax::mojom::blink::Role::kContentDeletion, "ContentDeletion"}, - {ax::mojom::blink::Role::kContentInsertion, "ContentInsertion"}, - {ax::mojom::blink::Role::kContentInfo, "ContentInfo"}, - {ax::mojom::blink::Role::kDate, "Date"}, - {ax::mojom::blink::Role::kDateTime, "DateTime"}, - {ax::mojom::blink::Role::kDefinition, "Definition"}, - {ax::mojom::blink::Role::kDescriptionListDetail, "DescriptionListDetail"}, - {ax::mojom::blink::Role::kDescriptionList, "DescriptionList"}, - {ax::mojom::blink::Role::kDescriptionListTerm, "DescriptionListTerm"}, - {ax::mojom::blink::Role::kDesktop, "Desktop"}, - {ax::mojom::blink::Role::kDetails, "Details"}, - {ax::mojom::blink::Role::kDialog, "Dialog"}, - {ax::mojom::blink::Role::kDirectory, "Directory"}, - {ax::mojom::blink::Role::kDisclosureTriangle, "DisclosureTriangle"}, - // -------------------------------------------------------------- - // DPub Roles: - // https://www.w3.org/TR/dpub-aam-1.0/#mapping_role_table - {ax::mojom::blink::Role::kDocAbstract, "DocAbstract"}, - {ax::mojom::blink::Role::kDocAcknowledgments, "DocAcknowledgments"}, - {ax::mojom::blink::Role::kDocAfterword, "DocAfterword"}, - {ax::mojom::blink::Role::kDocAppendix, "DocAppendix"}, - {ax::mojom::blink::Role::kDocBackLink, "DocBackLink"}, - {ax::mojom::blink::Role::kDocBiblioEntry, "DocBiblioentry"}, - {ax::mojom::blink::Role::kDocBibliography, "DocBibliography"}, - {ax::mojom::blink::Role::kDocBiblioRef, "DocBiblioref"}, - {ax::mojom::blink::Role::kDocChapter, "DocChapter"}, - {ax::mojom::blink::Role::kDocColophon, "DocColophon"}, - {ax::mojom::blink::Role::kDocConclusion, "DocConclusion"}, - {ax::mojom::blink::Role::kDocCover, "DocCover"}, - {ax::mojom::blink::Role::kDocCredit, "DocCredit"}, - {ax::mojom::blink::Role::kDocCredits, "DocCredits"}, - {ax::mojom::blink::Role::kDocDedication, "DocDedication"}, - {ax::mojom::blink::Role::kDocEndnote, "DocEndnote"}, - {ax::mojom::blink::Role::kDocEndnotes, "DocEndnotes"}, - {ax::mojom::blink::Role::kDocEpigraph, "DocEpigraph"}, - {ax::mojom::blink::Role::kDocEpilogue, "DocEpilogue"}, - {ax::mojom::blink::Role::kDocErrata, "DocErrata"}, - {ax::mojom::blink::Role::kDocExample, "DocExample"}, - {ax::mojom::blink::Role::kDocFootnote, "DocFootnote"}, - {ax::mojom::blink::Role::kDocForeword, "DocForeword"}, - {ax::mojom::blink::Role::kDocGlossary, "DocGlossary"}, - {ax::mojom::blink::Role::kDocGlossRef, "DocGlossref"}, - {ax::mojom::blink::Role::kDocIndex, "DocIndex"}, - {ax::mojom::blink::Role::kDocIntroduction, "DocIntroduction"}, - {ax::mojom::blink::Role::kDocNoteRef, "DocNoteref"}, - {ax::mojom::blink::Role::kDocNotice, "DocNotice"}, - {ax::mojom::blink::Role::kDocPageBreak, "DocPagebreak"}, - {ax::mojom::blink::Role::kDocPageFooter, "DocPageFooter"}, - {ax::mojom::blink::Role::kDocPageHeader, "DocPageHeader"}, - {ax::mojom::blink::Role::kDocPageList, "DocPagelist"}, - {ax::mojom::blink::Role::kDocPart, "DocPart"}, - {ax::mojom::blink::Role::kDocPreface, "DocPreface"}, - {ax::mojom::blink::Role::kDocPrologue, "DocPrologue"}, - {ax::mojom::blink::Role::kDocPullquote, "DocPullquote"}, - {ax::mojom::blink::Role::kDocQna, "DocQna"}, - {ax::mojom::blink::Role::kDocSubtitle, "DocSubtitle"}, - {ax::mojom::blink::Role::kDocTip, "DocTip"}, - {ax::mojom::blink::Role::kDocToc, "DocToc"}, - // End DPub roles. - // -------------------------------------------------------------- - {ax::mojom::blink::Role::kDocument, "Document"}, - {ax::mojom::blink::Role::kEmbeddedObject, "EmbeddedObject"}, - {ax::mojom::blink::Role::kEmphasis, "Emphasis"}, - {ax::mojom::blink::Role::kFeed, "feed"}, - {ax::mojom::blink::Role::kFigcaption, "Figcaption"}, - {ax::mojom::blink::Role::kFigure, "Figure"}, - {ax::mojom::blink::Role::kFooter, "Footer"}, - {ax::mojom::blink::Role::kFooterAsNonLandmark, "FooterAsNonLandmark"}, - {ax::mojom::blink::Role::kForm, "Form"}, - {ax::mojom::blink::Role::kGenericContainer, "GenericContainer"}, - // -------------------------------------------------------------- - // ARIA Graphics module roles: - // https://rawgit.com/w3c/graphics-aam/master/#mapping_role_table - {ax::mojom::blink::Role::kGraphicsDocument, "GraphicsDocument"}, - {ax::mojom::blink::Role::kGraphicsObject, "GraphicsObject"}, - {ax::mojom::blink::Role::kGraphicsSymbol, "GraphicsSymbol"}, - // End ARIA Graphics module roles. - // -------------------------------------------------------------- - {ax::mojom::blink::Role::kGrid, "Grid"}, - {ax::mojom::blink::Role::kGroup, "Group"}, - {ax::mojom::blink::Role::kHeader, "Header"}, - {ax::mojom::blink::Role::kHeaderAsNonLandmark, "HeaderAsNonLandmark"}, - {ax::mojom::blink::Role::kHeading, "Heading"}, - {ax::mojom::blink::Role::kIframePresentational, "IframePresentational"}, - {ax::mojom::blink::Role::kIframe, "Iframe"}, - {ax::mojom::blink::Role::kIgnored, "Ignored"}, - {ax::mojom::blink::Role::kImage, "Image"}, - {ax::mojom::blink::Role::kImeCandidate, "ImeCandidate"}, - {ax::mojom::blink::Role::kInlineTextBox, "InlineTextBox"}, - {ax::mojom::blink::Role::kInputTime, "InputTime"}, - {ax::mojom::blink::Role::kKeyboard, "Keyboard"}, - {ax::mojom::blink::Role::kLabelText, "Label"}, - {ax::mojom::blink::Role::kLayoutTable, "LayoutTable"}, - {ax::mojom::blink::Role::kLayoutTableCell, "LayoutCellTable"}, - {ax::mojom::blink::Role::kLayoutTableRow, "LayoutRowTable"}, - {ax::mojom::blink::Role::kLegend, "Legend"}, - {ax::mojom::blink::Role::kLink, "Link"}, - {ax::mojom::blink::Role::kLineBreak, "LineBreak"}, - {ax::mojom::blink::Role::kListBox, "ListBox"}, - {ax::mojom::blink::Role::kListBoxOption, "ListBoxOption"}, - {ax::mojom::blink::Role::kListGrid, "ListGrid"}, - {ax::mojom::blink::Role::kListItem, "ListItem"}, - {ax::mojom::blink::Role::kListMarker, "ListMarker"}, - {ax::mojom::blink::Role::kList, "List"}, - {ax::mojom::blink::Role::kLog, "Log"}, - {ax::mojom::blink::Role::kMain, "Main"}, - {ax::mojom::blink::Role::kMark, "Mark"}, - {ax::mojom::blink::Role::kMarquee, "Marquee"}, - {ax::mojom::blink::Role::kMath, "Math"}, - {ax::mojom::blink::Role::kMenuBar, "MenuBar"}, - {ax::mojom::blink::Role::kMenuItem, "MenuItem"}, - {ax::mojom::blink::Role::kMenuItemCheckBox, "MenuItemCheckBox"}, - {ax::mojom::blink::Role::kMenuItemRadio, "MenuItemRadio"}, - {ax::mojom::blink::Role::kMenuListOption, "MenuListOption"}, - {ax::mojom::blink::Role::kMenuListPopup, "MenuListPopup"}, - {ax::mojom::blink::Role::kMenu, "Menu"}, - {ax::mojom::blink::Role::kMeter, "Meter"}, - {ax::mojom::blink::Role::kNavigation, "Navigation"}, - {ax::mojom::blink::Role::kNote, "Note"}, - {ax::mojom::blink::Role::kPane, "Pane"}, - {ax::mojom::blink::Role::kParagraph, "Paragraph"}, - {ax::mojom::blink::Role::kPdfActionableHighlight, "PdfActionableHighlight"}, - {ax::mojom::blink::Role::kPdfRoot, "PdfRoot"}, - {ax::mojom::blink::Role::kPluginObject, "PluginObject"}, - {ax::mojom::blink::Role::kPopUpButton, "PopUpButton"}, - {ax::mojom::blink::Role::kPortal, "Portal"}, - {ax::mojom::blink::Role::kPre, "Pre"}, - {ax::mojom::blink::Role::kPresentational, "Presentational"}, - {ax::mojom::blink::Role::kProgressIndicator, "ProgressIndicator"}, - {ax::mojom::blink::Role::kRadioButton, "RadioButton"}, - {ax::mojom::blink::Role::kRadioGroup, "RadioGroup"}, - {ax::mojom::blink::Role::kRegion, "Region"}, - {ax::mojom::blink::Role::kRootWebArea, "WebArea"}, - {ax::mojom::blink::Role::kRow, "Row"}, - {ax::mojom::blink::Role::kRowGroup, "RowGroup"}, - {ax::mojom::blink::Role::kRowHeader, "RowHeader"}, - {ax::mojom::blink::Role::kRuby, "Ruby"}, - {ax::mojom::blink::Role::kRubyAnnotation, "RubyAnnotation"}, - {ax::mojom::blink::Role::kSection, "Section"}, - {ax::mojom::blink::Role::kSvgRoot, "SVGRoot"}, - {ax::mojom::blink::Role::kScrollBar, "ScrollBar"}, - {ax::mojom::blink::Role::kScrollView, "ScrollView"}, - {ax::mojom::blink::Role::kSearch, "Search"}, - {ax::mojom::blink::Role::kSearchBox, "SearchBox"}, - {ax::mojom::blink::Role::kSlider, "Slider"}, - {ax::mojom::blink::Role::kSpinButton, "SpinButton"}, - {ax::mojom::blink::Role::kSplitter, "Splitter"}, - {ax::mojom::blink::Role::kStaticText, "StaticText"}, - {ax::mojom::blink::Role::kStatus, "Status"}, - {ax::mojom::blink::Role::kStrong, "Strong"}, - {ax::mojom::blink::Role::kSuggestion, "Suggestion"}, - {ax::mojom::blink::Role::kSwitch, "Switch"}, - {ax::mojom::blink::Role::kTab, "Tab"}, - {ax::mojom::blink::Role::kTabList, "TabList"}, - {ax::mojom::blink::Role::kTabPanel, "TabPanel"}, - {ax::mojom::blink::Role::kTable, "Table"}, - {ax::mojom::blink::Role::kTableHeaderContainer, "TableHeaderContainer"}, - {ax::mojom::blink::Role::kTerm, "Term"}, - {ax::mojom::blink::Role::kTextField, "TextField"}, - {ax::mojom::blink::Role::kTextFieldWithComboBox, "ComboBox"}, - {ax::mojom::blink::Role::kTime, "Time"}, - {ax::mojom::blink::Role::kTimer, "Timer"}, - {ax::mojom::blink::Role::kTitleBar, "TitleBar"}, - {ax::mojom::blink::Role::kToggleButton, "ToggleButton"}, - {ax::mojom::blink::Role::kToolbar, "Toolbar"}, - {ax::mojom::blink::Role::kTreeGrid, "TreeGrid"}, - {ax::mojom::blink::Role::kTreeItem, "TreeItem"}, - {ax::mojom::blink::Role::kTree, "Tree"}, - {ax::mojom::blink::Role::kTooltip, "UserInterfaceTooltip"}, - {ax::mojom::blink::Role::kUnknown, "Unknown"}, - {ax::mojom::blink::Role::kVideo, "Video"}, - {ax::mojom::blink::Role::kWebView, "WebView"}, - {ax::mojom::blink::Role::kWindow, "Window"}}; - - static_assert(base::size(kInternalRoles) == - static_cast<size_t>(ax::mojom::blink::Role::kMaxValue) + 1, - "Not all internal roles have an entry in internalRoles array"); - - // Roles which we need to map in the other direction + // More friendly names for debugging. These are roles which don't map from + // the ARIA role name to the internal role when building the tree, but when + // debugging, we want to show the ARIA role name, since it is close in meaning. @@ -602:603 @@ - {"progressbar", ax::mojom::blink::Role::kMeter}, @@ -604:605 @@ - {"textbox", ax::mojom::blink::Role::kTextField}, @@ -611:613, +407:409 @@ - for (size_t i = 0; i < base::size(kRoles); ++i) - role_map->Set(String(kRoles[i].aria_role), kRoles[i].webcore_role); + for (auto aria_role : kAriaRoles) + role_map->Set(String(aria_role.role_name), aria_role.role); @@ -617:622, +413:417 @@ - static Vector<AtomicString>* CreateRoleNameVector() { - Vector<AtomicString>* role_name_vector = - new Vector<AtomicString>(base::size(kInternalRoles)); - for (wtf_size_t i = 0; i < base::size(kInternalRoles); i++) - (*role_name_vector)[i] = g_null_atom; + // The role name vector contains only ARIA roles, and no internal roles. + static Vector<AtomicString>* CreateARIARoleNameVector() { + Vector<AtomicString>* role_name_vector = new Vector<AtomicString>(kNumRoles); + role_name_vector->Fill(g_null_atom, kNumRoles); @@ -623:626, +418:421 @@ - for (wtf_size_t i = 0; i < base::size(kRoles); ++i) { - (*role_name_vector)[static_cast<wtf_size_t>(kRoles[i].webcore_role)] = - AtomicString(kRoles[i].aria_role); + for (auto aria_role : kAriaRoles) { + (*role_name_vector)[static_cast<wtf_size_t>(aria_role.role)] = + AtomicString(aria_role.role_name); @@ -628:632, +423:426 @@ - for (wtf_size_t i = 0; i < base::size(kReverseRoles); ++i) { - (*role_name_vector)[static_cast<wtf_size_t>( - kReverseRoles[i].webcore_role)] = - AtomicString(kReverseRoles[i].aria_role); + for (auto reverse_role : kReverseRoles) { + (*role_name_vector)[static_cast<wtf_size_t>(reverse_role.role)] = + AtomicString(reverse_role.role_name); @@ -637:649 @@ - static Vector<AtomicString>* CreateInternalRoleNameVector() { - Vector<AtomicString>* internal_role_name_vector = - new Vector<AtomicString>(base::size(kInternalRoles)); - for (wtf_size_t i = 0; i < base::size(kInternalRoles); i++) { - (*internal_role_name_vector)[static_cast<wtf_size_t>( - kInternalRoles[i].webcore_role)] = - AtomicString(kInternalRoles[i].internal_role_name); - } - - return internal_role_name_vector; - } - @@ -653:696 @@ - // TODO(dmazzoni): replace this with a call to RoleName(). - std::string GetEquivalentAriaRoleString(const ax::mojom::blink::Role role) { - switch (role) { - case ax::mojom::blink::Role::kArticle: - return "article"; - case ax::mojom::blink::Role::kBanner: - return "banner"; - case ax::mojom::blink::Role::kButton: - return "button"; - case ax::mojom::blink::Role::kComplementary: - return "complementary"; - case ax::mojom::blink::Role::kFigure: - return "figure"; - case ax::mojom::blink::Role::kFooter: - return "contentinfo"; - case ax::mojom::blink::Role::kHeader: - return "banner"; - case ax::mojom::blink::Role::kHeading: - return "heading"; - case ax::mojom::blink::Role::kImage: - return "img"; - case ax::mojom::blink::Role::kMain: - return "main"; - case ax::mojom::blink::Role::kNavigation: - return "navigation"; - case ax::mojom::blink::Role::kRadioButton: - return "radio"; - case ax::mojom::blink::Role::kRegion: - return "region"; - case ax::mojom::blink::Role::kSection: - // A <section> element uses the 'region' ARIA role mapping. - return "region"; - case ax::mojom::blink::Role::kSlider: - return "slider"; - case ax::mojom::blink::Role::kTime: - return "time"; - default: - break; - } - - return std::string(); - } - @@ -738:739 @@ - cached_has_inherited_presentational_role_(false), @@ -979:980, +717:718 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -988:989, +726:727 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -997:998, +735:736 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -1007:1008, +745:746 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -1016:1017, +754:755 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -1044:1045, +782:783 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -1056:1057, +794:795 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -1068:1069, +806:807 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -1080:1081, +818:819 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -1299:1313 @@ - if (const AtomicString& aria_role = - GetAOMPropertyOrARIAAttribute(AOMStringProperty::kRole)) { - TruncateAndAddStringAttribute(node_data, - ax::mojom::blink::StringAttribute::kRole, - aria_role.Utf8()); - } else { - std::string role_str = GetEquivalentAriaRoleString(RoleValue()); - if (!role_str.empty()) { - TruncateAndAddStringAttribute(node_data, - ax::mojom::blink::StringAttribute::kRole, - GetEquivalentAriaRoleString(RoleValue())); - } - } - @@ -1428:1429, +1152:1153 @@ - Element* element = this->GetElement(); + Element* element = GetElement(); @@ -1444:1448, +1168:1173 @@ - std::string role = GetEquivalentAriaRoleString(RoleValue()); - if (!role.empty()) { - TruncateAndAddStringAttribute( - node_data, ax::mojom::blink::StringAttribute::kRole, role); + const AtomicString& role_str = GetEquivalentAriaRoleName(RoleValue()); + if (role_str != g_null_atom) { + TruncateAndAddStringAttribute(node_data, + ax::mojom::blink::StringAttribute::kRole, + role_str.Ascii()); @@ -1759:1772 @@ - bool AXObject::IsCheckboxOrRadio() const { - switch (RoleValue()) { - case ax::mojom::blink::Role::kCheckBox: - case ax::mojom::blink::Role::kMenuItemCheckBox: - case ax::mojom::blink::Role::kMenuItemRadio: - case ax::mojom::blink::Role::kRadioButton: - return true; - default: - break; - } - return false; - } - @@ -1867:1868, +1579:1580 @@ - const Node* node = this->GetNode(); + const Node* node = GetNode(); @@ -2166:2168 @@ - cached_has_inherited_presentational_role_ = - !!InheritsPresentationalRoleFrom(); @@ -2688:2693 @@ - bool AXObject::HasInheritedPresentationalRole() const { - UpdateCachedAttributeValuesIfNeeded(); - return cached_has_inherited_presentational_role_; - } - @@ -3166:3167, +2871:2872 @@ - ElementsFromAttribute(elements_from_attribute, attr, ids); + ElementsFromAttribute(element, elements_from_attribute, attr, ids); @@ -3260:3264, +2965:2970 @@ - void AXObject::TokenVectorFromAttribute(Vector<String>& tokens, - const QualifiedName& attribute) const { - Node* node = this->GetNode(); - if (!node || !node->IsElementNode()) + // static + void AXObject::TokenVectorFromAttribute(Element* element, + Vector<String>& tokens, + const QualifiedName& attribute) { + if (!element) @@ -3266:3267, +2972:2973 @@ - String attribute_value = GetAttribute(attribute).GetString(); + String attribute_value = element->FastGetAttribute(attribute).GetString(); @@ -3274:3275, +2980:2983 @@ - void AXObject::ElementsFromAttribute(HeapVector<Member<Element>>& elements, + // static + bool AXObject::ElementsFromAttribute(Element* from, + HeapVector<Member<Element>>& elements, @@ -3276:3277, +2984:2988 @@ - Vector<String>& ids) const { + Vector<String>& ids) { + if (!from) + return false; + @@ -3279:3283, +2990:2991 @@ - TokenVectorFromAttribute(ids, attribute); - Element* element = GetElement(); - if (!element) - return; + TokenVectorFromAttribute(from, ids, attribute); @@ -3285:3286, +2993:2994 @@ - element->GetElementArrayAttribute(attribute); + from->GetElementArrayAttribute(attribute); @@ -3287:3288, +2995:2996 @@ - return; + return false; @@ +2999:3001 @@ + + return elements.size(); @@ -3293:3294, +3003:3006 @@ - void AXObject::AriaLabelledbyElementVector( + // static + bool AXObject::AriaLabelledbyElementVector( + Element* from, @@ -3295:3296, +3007:3008 @@ - Vector<String>& ids) const { + Vector<String>& ids) { @@ -3297:3300, +3009:3016 @@ - ElementsFromAttribute(elements, html_names::kAriaLabelledbyAttr, ids); - if (!ids.size()) - ElementsFromAttribute(elements, html_names::kAriaLabeledbyAttr, ids); + if (ElementsFromAttribute(from, elements, html_names::kAriaLabelledbyAttr, + ids)) { + return true; + } + + return ElementsFromAttribute(from, elements, html_names::kAriaLabeledbyAttr, + ids); @@ -3306:3307, +3022:3023 @@ - AriaLabelledbyElementVector(elements, ids); + AriaLabelledbyElementVector(GetElement(), elements, ids); @@ +3029:3030 @@ + @@ -3314:3315, +3031:3033 @@ - ElementsFromAttribute(elements, html_names::kAriaDescribedbyAttr, ids); + ElementsFromAttribute(GetElement(), elements, + html_names::kAriaDescribedbyAttr, ids); @@ -3464:3512, +3182:3221 @@ - bool IsGlobalARIAAttribute(const AtomicString& name) { - if (!name.StartsWith("ARIA")) - return false; - if (name.StartsWith("ARIA-ATOMIC")) - return true; - if (name.StartsWith("ARIA-BUSY")) - return true; - if (name.StartsWith("ARIA-CONTROLS")) - return true; - if (name.StartsWith("ARIA-CURRENT")) - return true; - if (name.StartsWith("ARIA-DESCRIBEDBY")) - return true; - if (name.StartsWith("ARIA-DETAILS")) - return true; - if (name.StartsWith("ARIA-DISABLED")) - return true; - if (name.StartsWith("ARIA-DROPEFFECT")) - return true; - if (name.StartsWith("ARIA-ERRORMESSAGE")) - return true; - if (name.StartsWith("ARIA-FLOWTO")) - return true; - if (name.StartsWith("ARIA-GRABBED")) - return true; - if (name.StartsWith("ARIA-HASPOPUP")) - return true; - if (name.StartsWith("ARIA-HIDDEN")) - return true; - if (name.StartsWith("ARIA-INVALID")) - return true; - if (name.StartsWith("ARIA-KEYSHORTCUTS")) - return true; - if (name.StartsWith("ARIA-LABEL")) - return true; - if (name.StartsWith("ARIA-LABELEDBY")) - return true; - if (name.StartsWith("ARIA-LABELLEDBY")) - return true; - if (name.StartsWith("ARIA-LIVE")) - return true; - if (name.StartsWith("ARIA-OWNS")) - return true; - if (name.StartsWith("ARIA-RELEVANT")) - return true; - if (name.StartsWith("ARIA-ROLEDESCRIPTION")) - return true; - return false; + bool DoesUndoRolePresentation(const AtomicString& name) { + // This is the list of global ARIA properties that force + // role="presentation"/"none" to be exposed, and does not contain ARIA + // properties who's global status is being deprecated. + // TODO(accessibility) aria-label/labelledby is not allowed on + // role="none"/"presentation", therefore it should not be listed here. + // However, this breaks a few name tests that assume aria-label causes + // role="none"/"presentation" to be exposed, even though the property is + // prohibited there. See https://github.com/w3c/accname/issues/121. + // clang-format off + DEFINE_STATIC_LOCAL( + HashSet<AtomicString>, aria_global_properties, + ({ + "ARIA-ATOMIC", + // TODO(accessibility/ARIA 1.3) Add these (and test in aria-global.html) + // "ARIA-BRAILLELABEL", + // "ARIA-BRAILLEROLEDESCRIPTION", + "ARIA-BUSY", + "ARIA-CONTROLS", + "ARIA-CURRENT", + "ARIA-DESCRIBEDBY", + "ARIA-DESCRIPTION", + "ARIA-DETAILS", + "ARIA-DROPEFFECT", + "ARIA-FLOWTO", + "ARIA-GRABBED", + "ARIA-HIDDEN", // For aria-hidden=false. + "ARIA-KEYSHORTCUTS", + "ARIA-LABEL", + "ARIA-LABELEDBY", + "ARIA-LABELLEDBY", + "ARIA-LIVE", + "ARIA-OWNS", + "ARIA-RELEVANT", + "ARIA-ROLEDESCRIPTION" + })); + // clang-format on + + return aria_global_properties.Contains(name); @@ -3514:3515, +3223:3224 @@ - bool AXObject::HasGlobalARIAAttribute() const { + bool AXObject::HasAriaAttribute(bool does_undo_role_presentation) const { @@ +3228:3235 @@ + // A role is considered an ARIA attribute. + if (!does_undo_role_presentation && + AriaRoleAttribute() != ax::mojom::blink::Role::kUnknown) { + return true; + } + + // Check for any attribute that begins with "aria-". @@ -3523:3525, +3239:3243 @@ - if (IsGlobalARIAAttribute(name)) - return true; + if (name.StartsWith("ARIA-")) { + if (!does_undo_role_presentation || DoesUndoRolePresentation(name)) + return true; + } @@ -3526:3534, +3244:3245 @@ - if (!element->DidAttachInternals()) - return false; - const auto& internals_attributes = - element->EnsureElementInternals().GetAttributes(); - for (const QualifiedName& attr : internals_attributes.Keys()) { - if (IsGlobalARIAAttribute(attr.LocalNameUpper())) - return true; - } + @@ -3646:3647, +3357:3358 @@ - ax::mojom::blink::Role role = AriaRoleToWebCoreRole(aria_role); + ax::mojom::blink::Role role = AriaRoleStringToRoleEnum(aria_role); @@ -3655:3658, +3366:3373 @@ - HasGlobalARIAAttribute()) { - // If we return an unknown role, then the native HTML role would be used - // instead. + HasAriaAttribute(true /* does_undo_role_presentation */)) { + // Must be exposed with a role if focusable or has a global ARIA property + // that is allowed in this context. See + // https://w3c.github.io/aria/#presentation for more information about the + // conditions upon which elements with role="none"/"presentation" must be + // included in the tree. Return Role::kUnknown, so that the native HTML + // role is used instead. @@ -3707:3719 @@ - if (!GetElement() || !GetElement()->IsInCanvasSubtree()) - return false; - - for (const Node* n = node; n; n = n->parentNode()) { - if (const AXObject* obj = AXObjectCache().Get(n)) - return obj->IsNativeTextField() || obj->HasContentEditableAttributeSet(); - - // Editable states do not cross document boundaries. - if (node->GetDocument().body() == n) - break; - } - @@ -3723:3757, +3426:3427 @@ - const Node* node = GetNode(); - if (IsDetached() || !node) - return false; - #if DCHECK_IS_ON() // Required in order to get Lifecycle().ToString() - DCHECK(GetDocument()); - DCHECK_GE(GetDocument()->Lifecycle().GetState(), - DocumentLifecycle::kStyleClean) - << "Unclean document style at lifecycle state " - << GetDocument()->Lifecycle().ToString(); - #endif // DCHECK_IS_ON() - - // The DOM inside native text fields is an implementation detail that should - // not be exposed to platform accessibility APIs. - if (EnclosingTextControl(node)) - return false; - - if (IsRootEditableElement(*node)) - return true; - - // Catches the case where a contenteditable is inside another contenteditable. - // This is especially important when the two nested contenteditables have - // different attributes, e.g. "true" vs. "plaintext-only". Also, catches the - // case where this object is in a canvas subtree. - if (HasContentEditableAttributeSet()) - return true; - - // In case the object is a body element in design mode, - // `blink::IsRootEditableElement` should have returned true above. However, - // this won't happen when in a canvas subtree. - if (!GetElement() || !GetElement()->IsInCanvasSubtree()) - return false; - - return node->GetDocument().InDesignMode() && - node->GetDocument().documentElement() == node; + return false; @@ -3760:3769, +3430:3431 @@ - if (IsDetached() || !GetNode()) - return false; - - const auto* html_element = DynamicTo<HTMLElement>(GetNode()); - if (!html_element) - return false; - - String normalized_value = html_element->contentEditable(); - return normalized_value == "true" || normalized_value == "plaintext-only"; + return false; @@ -3800:3812 @@ - if (!GetElement() || !GetElement()->IsInCanvasSubtree()) - return false; - - for (const Node* n = node; n; n = n->parentNode()) { - if (const AXObject* obj = AXObjectCache().Get(n)) - return obj->HasContentEditableAttributeSet(); - - // Editable states do not cross document boundaries. - if (node->GetDocument().body() == n) - break; - } - @@ -5114:5115, +4764:4765 @@ - bool AXObject::InternalClearAccessibilityFocusAction() { + bool AXObject::InternalSetAccessibilityFocusAction() { @@ -5118:5119, +4768:4769 @@ - bool AXObject::InternalSetAccessibilityFocusAction() { + bool AXObject::InternalClearAccessibilityFocusAction() { @@ -5275:5278, +4925:4928 @@ - // TODO: do we need to check !AriaOwnsElements.empty() ? Is that fundamentally - // different from HasExplicitlySetAttrAssociatedElements()? And is an element - // even necessary in the case of virtual nodes? + // TODO(accessibility): do we need to check !AriaOwnsElements.empty() ? Is + // that fundamentally different from HasExplicitlySetAttrAssociatedElements()? + // And is an element even necessary in the case of virtual nodes? @@ -5283:5284, +4933:4934 @@ - ax::mojom::blink::Role AXObject::AriaRoleToWebCoreRole(const String& value) { + ax::mojom::blink::Role AXObject::AriaRoleStringToRoleEnum(const String& value) { @@ -5603:5605, +5253:5256 @@ - const AtomicString& AXObject::RoleName(ax::mojom::blink::Role role) { - static const Vector<AtomicString>* role_name_vector = CreateRoleNameVector(); + const AtomicString& AXObject::ARIARoleName(ax::mojom::blink::Role role) { + static const Vector<AtomicString>* aria_role_name_vector = + CreateARIARoleNameVector(); @@ -5606:5607, +5257:5258 @@ - return role_name_vector->at(static_cast<wtf_size_t>(role)); + return aria_role_name_vector->at(static_cast<wtf_size_t>(role)); @@ -5610:5613, +5261:5288 @@ - const AtomicString& AXObject::InternalRoleName(ax::mojom::blink::Role role) { - static const Vector<AtomicString>* internal_role_name_vector = - CreateInternalRoleNameVector(); + const AtomicString& AXObject::GetEquivalentAriaRoleName( + const ax::mojom::blink::Role role) { + // TODO(accessibilty) Why are some roles listed here and not others? + switch (role) { + case ax::mojom::blink::Role::kSection: + // A <section> element uses the 'region' ARIA role mapping. + return ARIARoleName(ax::mojom::blink::Role::kRegion); + case ax::mojom::blink::Role::kArticle: + case ax::mojom::blink::Role::kBanner: + case ax::mojom::blink::Role::kButton: + case ax::mojom::blink::Role::kComplementary: + case ax::mojom::blink::Role::kFigure: + case ax::mojom::blink::Role::kFooter: + case ax::mojom::blink::Role::kHeader: + case ax::mojom::blink::Role::kHeading: + case ax::mojom::blink::Role::kImage: + case ax::mojom::blink::Role::kMain: + case ax::mojom::blink::Role::kNavigation: + case ax::mojom::blink::Role::kRadioButton: + case ax::mojom::blink::Role::kRegion: + case ax::mojom::blink::Role::kSlider: + case ax::mojom::blink::Role::kTime: + return ARIARoleName(role); + default: + return g_null_atom; + } + } @@ -5614:5615, +5289:5312 @@ - return internal_role_name_vector->at(static_cast<wtf_size_t>(role)); + const String AXObject::InternalRoleName(ax::mojom::blink::Role role) { + std::ostringstream role_name; + role_name << role; + // Convert from std::ostringstream to std::string, while removing "k" prefix. + // For example, kStaticText becomes StaticText. + // Many conversions, but this isn't used in performance-sensitive code. + std::string role_name_std = role_name.str().substr(1, std::string::npos); + String role_name_wtf_string = role_name_std.c_str(); + return role_name_wtf_string; + } + + // static + const String AXObject::RoleName(ax::mojom::blink::Role role, + bool* is_internal) { + if (is_internal) + *is_internal = false; + if (const auto& role_name = ARIARoleName(role)) + return role_name.GetString(); + + if (is_internal) + *is_internal = true; + + return InternalRoleName(role); @@ -5663:5665, +5360:5361 @@ - String string_builder = - AXObject::InternalRoleName(RoleValue()).GetString().EncodeForDebugging(); + String string_builder = InternalRoleName(RoleValue()).EncodeForDebugging(); ``` The name of the file: third_party/blink/renderer/modules/accessibility/ax_object.h Insertions: 6, Deletions: 2. ``` @@ -143:144, +143:144 @@ - ax::mojom::blink::DescriptionFrom::kUninitialized; + ax::mojom::blink::DescriptionFrom::kNone; @@ -403:404, +403:406 @@ - void TokenVectorFromAttribute(Vector<String>&, const QualifiedName&) const; + static void TokenVectorFromAttribute(Element* element, + Vector<String>&, + const QualifiedName&); @@ -440:441 @@ - bool IsCheckboxOrRadio() const; @@ -567:568 @@ - bool HasInheritedPresentationalRole() const; @@ -639:646 @@ - // Internal function used to determine whether the result of calling |GetName| - // on this object would return text that came from the an HTML label element - // or not. This is intended to be faster than calling |GetName| or - // |TextAlternative|, and without side effects (it won't call - // AXObjectCache->GetOrCreate). - virtual bool NameFromLabelElement() const { return false; } - @@ -824:825, +817:818 @@ - virtual bool HasAriaAttribute() const { return false; } + bool HasAriaAttribute(bool does_undo_role_presentation = false) const; @@ -848:849, +841:842 @@ - bool IsEditableRoot() const; + virtual bool IsEditableRoot() const; @@ -852:853, +845:846 @@ - bool HasContentEditableAttributeSet() const; + virtual bool HasContentEditableAttributeSet() const; @@ -865:866 @@ - bool HasGlobalARIAAttribute() const; @@ +1155:1159 @@ + // + // If this object is associated with generated content, or a list marker, + // returns a pseudoelement. It does not return the node that generated the + // content or the list marker. @@ -1258:1260, +1254:1256 @@ - bool InternalClearAccessibilityFocusAction(); - bool InternalSetAccessibilityFocusAction(); + virtual bool InternalSetAccessibilityFocusAction(); + virtual bool InternalClearAccessibilityFocusAction(); @@ -1292:1295, +1288:1306 @@ - static ax::mojom::blink::Role AriaRoleToWebCoreRole(const String&); - static const AtomicString& RoleName(ax::mojom::blink::Role); - static const AtomicString& InternalRoleName(ax::mojom::blink::Role); + static ax::mojom::blink::Role AriaRoleStringToRoleEnum(const String&); + + // Return the equivalent ARIA name for an enumerated role, or g_null_atom. + static const AtomicString& ARIARoleName(ax::mojom::blink::Role); + + // For a native role get the equivalent ARIA role for use in the xml-roles + // object attribute. + static const AtomicString& GetEquivalentAriaRoleName(ax::mojom::blink::Role); + + // Return the equivalent internal role name as a string. + static const String InternalRoleName(ax::mojom::blink::Role); + + // Return a role name, preferring the ARIA over the internal name. + // Optional boolean out param |*is_internal| will be false if the role matches + // an ARIA role, and true if an internal role name is used (no ARIA mapping). + static const String RoleName(ax::mojom::blink::Role, + bool* is_internal = nullptr); + @@ -1356:1361, +1367:1374 @@ - void ElementsFromAttribute(HeapVector<Member<Element>>& elements, - const QualifiedName&, - Vector<String>& ids) const; - void AriaLabelledbyElementVector(HeapVector<Member<Element>>& elements, - Vector<String>& ids) const; + static bool ElementsFromAttribute(Element* from, + HeapVector<Member<Element>>& elements, + const QualifiedName&, + Vector<String>& ids); + static bool AriaLabelledbyElementVector(Element* from, + HeapVector<Member<Element>>& elements, + Vector<String>& ids); @@ -1366:1369 @@ - virtual const AXObject* InheritsPresentationalRoleFrom() const { - return nullptr; - } @@ -1415:1416 @@ - mutable bool cached_has_inherited_presentational_role_ : 1; ``` The name of the file: third_party/blink/renderer/modules/accessibility/ax_object_test.cc Insertions: 39, Deletions: 16. ``` @@ +6:8 @@ + #include <memory> + @@ -161:162, +163:164 @@ - <div contenteditable="true" id="contenteditable"> + <div contenteditable="true" id="outerContenteditable"> @@ +165:168 @@ + <div contenteditable="plaintext-only" id="innerContenteditable"> + Test + </div> @@ -176:180, +181:187 @@ - const AXObject* contenteditable = GetAXObjectByElementId("contenteditable"); - ASSERT_NE(nullptr, contenteditable); - const AXObject* contenteditable_text = contenteditable->UnignoredChildAt(0); - ASSERT_NE(nullptr, contenteditable_text); + const AXObject* outer_contenteditable = + GetAXObjectByElementId("outerContenteditable"); + ASSERT_NE(nullptr, outer_contenteditable); + const AXObject* outer_contenteditable_text = + outer_contenteditable->UnignoredChildAt(0); + ASSERT_NE(nullptr, outer_contenteditable_text); @@ -181:182, +188:197 @@ - contenteditable_text->RoleValue()); + outer_contenteditable_text->RoleValue()); + const AXObject* inner_contenteditable = + GetAXObjectByElementId("innerContenteditable"); + ASSERT_NE(nullptr, inner_contenteditable); + const AXObject* inner_contenteditable_text = + inner_contenteditable->UnignoredChildAt(0); + ASSERT_NE(nullptr, inner_contenteditable_text); + ASSERT_EQ(ax::mojom::blink::Role::kStaticText, + inner_contenteditable_text->RoleValue()); @@ -187:189, +202:206 @@ - EXPECT_TRUE(contenteditable->IsEditable()); - EXPECT_TRUE(contenteditable_text->IsEditable()); + EXPECT_TRUE(outer_contenteditable->IsEditable()); + EXPECT_TRUE(outer_contenteditable_text->IsEditable()); + EXPECT_TRUE(inner_contenteditable->IsEditable()); + EXPECT_TRUE(inner_contenteditable_text->IsEditable()); @@ -194:196, +211:215 @@ - EXPECT_TRUE(contenteditable->IsEditableRoot()); - EXPECT_FALSE(contenteditable_text->IsEditableRoot()); + EXPECT_TRUE(outer_contenteditable->IsEditableRoot()); + EXPECT_FALSE(outer_contenteditable_text->IsEditableRoot()); + EXPECT_TRUE(inner_contenteditable->IsEditableRoot()); + EXPECT_FALSE(inner_contenteditable_text->IsEditableRoot()); @@ -201:203, +220:224 @@ - EXPECT_TRUE(contenteditable->HasContentEditableAttributeSet()); - EXPECT_FALSE(contenteditable_text->HasContentEditableAttributeSet()); + EXPECT_TRUE(outer_contenteditable->HasContentEditableAttributeSet()); + EXPECT_FALSE(outer_contenteditable_text->HasContentEditableAttributeSet()); + EXPECT_TRUE(inner_contenteditable->HasContentEditableAttributeSet()); + EXPECT_FALSE(inner_contenteditable_text->HasContentEditableAttributeSet()); @@ -208:210, +229:233 @@ - EXPECT_TRUE(contenteditable->IsMultiline()); - EXPECT_FALSE(contenteditable_text->IsMultiline()); + EXPECT_TRUE(outer_contenteditable->IsMultiline()); + EXPECT_FALSE(outer_contenteditable_text->IsMultiline()); + EXPECT_TRUE(inner_contenteditable->IsMultiline()); + EXPECT_FALSE(inner_contenteditable_text->IsMultiline()); @@ -215:217, +238:242 @@ - EXPECT_TRUE(contenteditable->IsRichlyEditable()); - EXPECT_TRUE(contenteditable_text->IsRichlyEditable()); + EXPECT_TRUE(outer_contenteditable->IsRichlyEditable()); + EXPECT_TRUE(outer_contenteditable_text->IsRichlyEditable()); + EXPECT_FALSE(inner_contenteditable->IsRichlyEditable()); + EXPECT_FALSE(inner_contenteditable_text->IsRichlyEditable()); @@ -931:932, +956:957 @@ - ax_context_.reset(new AXContext(GetDocument())); + ax_context_ = std::make_unique<AXContext>(GetDocument()); @@ -957:958, +982:983 @@ - ax_context_.reset(new AXContext(GetDocument())); + ax_context_ = std::make_unique<AXContext>(GetDocument()); ```

              To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

              Gerrit-Project: chromium/src
              Gerrit-Branch: main
              Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
              Gerrit-Change-Number: 2780362
              Gerrit-PatchSet: 42
              Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
              Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
              Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
              Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
              Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
              Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
              Gerrit-CC: Alice Boxhall <abox...@chromium.org>
              Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
              Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
              Gerrit-MessageType: merged

              Findit (Gerrit)

              unread,
              Apr 23, 2021, 3:34:35 AM4/23/21
              to Nektarios Paisios, Chromium LUCI CQ, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Dominic Mazzoni, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt


              Findit (https://goo.gl/kROfz5) identified this CL at revision 875394 as the culprit for
              failures in the build cycles as shown on:
              https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzRiOGFhY2I0NzIwZjQ4NzNmYzg1NTQ5ZWRmOTRlNTIxMjkyMWJkNmYM
              If it is a false positive, please report it at https://bugs.chromium.org/p/chromium/issues/entry?status=Available&comment=Link+to+the+culprit%3A+https%3A%2F%2Fanalysis.chromium.org%2Fwaterfall%2Fculprit%3Fkey%3Dag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzRiOGFhY2I0NzIwZjQ4NzNmYzg1NTQ5ZWRmOTRlNTIxMjkyMWJkNmYM&labels=Test-Findit-Wrong&components=Tools%3ETest%3EFindIt&summary=Wrongly+blame+4b8aacb4720f4873fc85549edf94e5212921bd6f.

              View Change

                To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
                Gerrit-Change-Number: 2780362
                Gerrit-PatchSet: 42
                Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
                Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
                Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
                Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
                Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
                Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
                Gerrit-CC: Alice Boxhall <abox...@chromium.org>
                Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
                Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
                Gerrit-Comment-Date: Fri, 23 Apr 2021 07:34:23 +0000

                Christian Dullweber (Gerrit)

                unread,
                Apr 23, 2021, 4:54:55 AM4/23/21
                to Nektarios Paisios, Chromium LUCI CQ, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Findit, Dominic Mazzoni, Aaron Leventhal, Alice Boxhall, chromium...@chromium.org, Jeongeun Kim, Kevin Babbitt

                Christian Dullweber has created a revert of this change.

                View Change

                To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
                Gerrit-Change-Number: 2780362
                Gerrit-PatchSet: 42
                Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
                Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
                Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
                Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
                Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
                Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
                Gerrit-CC: Alice Boxhall <abox...@chromium.org>
                Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
                Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
                Gerrit-MessageType: revert

                Nektarios Paisios (Gerrit)

                unread,
                Sep 22, 2022, 1:51:24 PM9/22/22
                to Chromium LUCI CQ, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, mlamouri+wa...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Findit, Aaron Leventhal, chromium...@chromium.org, (Julie)Jeongeun Kim, Kevin Babbitt

                View Change

                1 comment:

                To view, visit change 2780362. To unsubscribe, or for help writing mail filters, visit settings.

                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: I31b1ddac4bc045311a4c14cf84e2d8d06460cbc7
                Gerrit-Change-Number: 2780362
                Gerrit-PatchSet: 42
                Gerrit-Owner: Nektarios Paisios <nek...@chromium.org>
                Gerrit-Reviewer: Aaron Leventhal <aleve...@chromium.org>
                Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
                Gerrit-Reviewer: Dominic Mazzoni <dmaz...@chromium.org>
                Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
                Gerrit-CC: (Julie)Jeongeun Kim <je_jul...@chromium.org>
                Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
                Gerrit-CC: Alice Boxhall <abox...@chromium.org>
                Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
                Gerrit-Comment-Date: Thu, 22 Sep 2022 17:51:00 +0000
                Gerrit-HasComments: Yes
                Gerrit-Has-Labels: No
                Gerrit-MessageType: comment
                Reply all
                Reply to author
                Forward
                0 new messages