Changeset 137512 in webkit


Ignore:
Timestamp:
Dec 12, 2012 1:50:51 PM (11 years ago)
Author:
dmazzoni@google.com
Message:

AX: textUnderElement should consider alt text, but skip links and controls
​https://bugs.webkit.org/show_bug.cgi?id=101650

Reviewed by Chris Fleizach.

Source/WebCore:

Getting inner text from an element now ignores focusable descendants and
containers, but uses alternative text.

The computation of textUnderElement is now recursive. This caused a crash
if it was called by accessibilityIsIgnored during object destruction,
so I simplified accessibilityIsIgnored to not actually call textUnderElement,
without affecting test results, which should actually be a decent speedup.

Test: accessibility/button-title-uses-inner-img-alt.html
Test: accessibility/focusable-div.html

  • accessibility/AccessibilityNodeObject.cpp:

(WebCore):
(WebCore::shouldUseAccessiblityObjectInnerText):
(WebCore::AccessibilityNodeObject::textUnderElement):

  • accessibility/AccessibilityRenderObject.cpp:

(WebCore::AccessibilityRenderObject::textUnderElement):

LayoutTests:

Adds new tests to show that getting inner text from an element now ignores
focusable descendants and containers, but uses alternative text.

Updates and rebaselines several tests to reflect the new logic.

  • accessibility/button-title-uses-inner-img-alt-expected.txt: Added.
  • accessibility/button-title-uses-inner-img-alt.html: Added.
  • accessibility/focusable-div-expected.txt: Extended with more test cases.
  • accessibility/focusable-div.html: Extended with more test cases.
  • platform/chromium/TestExpectations: Un-skip test that now passes.
  • platform/chromium/accessibility/image-link-expected.txt: Rebaseline
  • platform/mac/accessibility/image-link-expected.txt: Rebaseline
  • platform/mac/accessibility/internal-link-anchors2-expected.txt: Rebaseline
  • platform/mac/accessibility/static-text-role-uses-text-under-element-expected.txt: Rebaseline
  • platform/mac/accessibility/static-text-role-uses-text-under-element.html: Fix
  • platform/mac/accessibility/table-with-aria-role-expected.txt: Rebaseline
Location:
trunk
Files:
2 added
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r137509 r137512  
     12012-12-12  Dominic Mazzoni  <dmazzoni@google.com>
     2
     3        AX: textUnderElement should consider alt text, but skip links and controls
     4        https://bugs.webkit.org/show_bug.cgi?id=101650
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Adds new tests to show that getting inner text from an element now ignores
     9        focusable descendants and containers, but uses alternative text.
     10
     11        Updates and rebaselines several tests to reflect the new logic.
     12
     13        * accessibility/button-title-uses-inner-img-alt-expected.txt: Added.
     14        * accessibility/button-title-uses-inner-img-alt.html: Added.
     15        * accessibility/focusable-div-expected.txt: Extended with more test cases.
     16        * accessibility/focusable-div.html: Extended with more test cases.
     17        * platform/chromium/TestExpectations: Un-skip test that now passes.
     18        * platform/chromium/accessibility/image-link-expected.txt: Rebaseline
     19        * platform/mac/accessibility/image-link-expected.txt: Rebaseline
     20        * platform/mac/accessibility/internal-link-anchors2-expected.txt: Rebaseline
     21        * platform/mac/accessibility/static-text-role-uses-text-under-element-expected.txt: Rebaseline
     22        * platform/mac/accessibility/static-text-role-uses-text-under-element.html: Fix
     23        * platform/mac/accessibility/table-with-aria-role-expected.txt: Rebaseline
     24
    1252012-12-12  Philip Rogers  <pdr@google.com>
    226

  • trunk/LayoutTests/accessibility/focusable-div-expected.txt

    r126970 r137512  
    22B
    33C
    4 This test makes sure that a generic focusable div can get accessibility focus.
     4Link
     5Initial text before linkLink
     6List item
     7Initial text before list
     8List item
     9This test makes sure that a generic focusable div can get accessibility focus and gets its accessible text from contents..
    510
    611On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
    … …  
    1520PASS document.activeElement == div3 is true
    1621PASS lastChar(axDiv3.description) is "D"
     22PASS document.activeElement == div4 is true
     23PASS axDiv4.title.indexOf('Link') is -1
     24PASS document.activeElement == div5 is true
     25PASS axDiv5.title.indexOf('Link') is -1
     26PASS axDiv5.title.indexOf('Initial text before link') >= 0 is true
     27PASS document.activeElement == div6 is true
     28PASS axDiv6.title.indexOf('List item') is -1
     29PASS document.activeElement == div7 is true
     30PASS axDiv7.title.indexOf('List item') is -1
     31PASS axDiv7.title.indexOf('Initial text before list') >= 0 is true
    1732PASS successfullyParsed is true
    1833

  • trunk/LayoutTests/accessibility/focusable-div.html

    r126970 r137512  
    44<script src="../fast/js/resources/js-test-pre.js"></script>
    55
     6<!-- A link always gets its accessible text from contents. -->
    67<a id="link" href="#">A</a>
     8
     9<!-- A generic focusable div should also get its accessible text from contents. -->
    710<div id="div" tabindex="0">B</div>
    811<div id="div2" tabindex="0"><div></div>C</div>
    912<div id="div3" tabindex="0" aria-label="D"></div>
    1013
     14<!-- A generic focusable div should not get accessible text from children that are focusable or containers. -->
     15<div id="div4" tabindex="0"><a href="#">Link</a></div>
     16<div id="div5" tabindex="0">Initial text before link<a href="#">Link</a></div>
     17<div id="div6" tabindex="0"><ul><li>List item</li></ul></div>
     18<div id="div7" tabindex="0">Initial text before list<ul><li>List item</li></ul></div>
     19
    1120<div id="console"></div>
    1221<script>
    13 description("This test makes sure that a generic focusable div can get accessibility focus.");
     22description("This test makes sure that a generic focusable div can get accessibility focus and gets its accessible text from contents..");
    1423
    1524if (window.testRunner && window.accessibilityController) {
    … …  
    4352    window.axDiv3 = accessibilityController.focusedElement;
    4453    shouldBe("lastChar(axDiv3.description)", "\"D\"");
     54
     55    var div4 = document.getElementById('div4');
     56    div4.focus();
     57    shouldBe("document.activeElement == div4", "true");
     58    window.axDiv4 = accessibilityController.focusedElement;
     59    shouldBe("axDiv4.title.indexOf('Link')", "-1");
     60
     61    var div5 = document.getElementById('div5');
     62    div5.focus();
     63    shouldBe("document.activeElement == div5", "true");
     64    window.axDiv5 = accessibilityController.focusedElement;
     65    shouldBe("axDiv5.title.indexOf('Link')", "-1");
     66    shouldBe("axDiv5.title.indexOf('Initial text before link') >= 0", "true");
     67
     68    var div6 = document.getElementById('div6');
     69    div6.focus();
     70    shouldBe("document.activeElement == div6", "true");
     71    window.axDiv6 = accessibilityController.focusedElement;
     72    shouldBe("axDiv6.title.indexOf('List item')", "-1");
     73
     74    var div7 = document.getElementById('div7');
     75    div7.focus();
     76    shouldBe("document.activeElement == div7", "true");
     77    window.axDiv7 = accessibilityController.focusedElement;
     78    shouldBe("axDiv7.title.indexOf('List item')", "-1");
     79    shouldBe("axDiv7.title.indexOf('Initial text before list') >= 0", "true");
    4580}
    4681

  • trunk/LayoutTests/platform/chromium/TestExpectations

    r137504 r137512  
    13971397crbug.com/10322 accessibility/document-attributes.html [ Skip ]
    13981398crbug.com/10322 accessibility/iframe-bastardization.html [ Skip ]
    1399 crbug.com/10322 accessibility/image-link.html [ Skip ]
    14001399crbug.com/10322 accessibility/image-map-update-parent-crash.html [ Skip ]
    14011400crbug.com/10322 accessibility/image-map2.html [ Skip ]

  • trunk/LayoutTests/platform/chromium/accessibility/image-link-expected.txt

    r131683 r137512  
    44
    55
     6AXTitle: Delicious cake
    67AXRole: AXLink
    7 AXSubrole: (null)
    8 AXRoleDescription: link
    9 AXChildren: <array of size 1>
    10 AXHelp:
    11 AXParent: <AXLink>
    12 AXSize: NSSize: {280, 213}
     8AXDescription:
     9
     10Child 0:
    1311AXTitle:
    14 AXDescription:
    15 AXValue:
    16 AXFocused: 1
    17 AXEnabled: 1
    18 AXWindow: <AXLink>
    19 AXSelectedTextMarkerRange: (null)
    20 AXStartTextMarker: <AXLink>
    21 AXEndTextMarker: <AXLink>
    22 AXVisited: 0
    23 AXLinkedUIElements: (null)
    24 AXSelected: 0
    25 AXBlockQuoteLevel: 0
    26 AXTopLevelUIElement: <AXLink>
    27 AXURL: http://www.wowhead.com/?item=33924
    28 AXAccessKey: (null)
     12AXRole: AXImage
     13AXDescription: Delicious cake
    2914
    3015
    31 Child 0:
    32 AXRole: AXImage
    33 AXSubrole: (null)
    34 AXRoleDescription: image
    35 AXChildren: <array of size 0>
    36 AXHelp:
    37 AXParent: <AXImage>
    38 AXSize: NSSize: {280, 209}
    39 AXTitle:
    40 AXDescription: Delicious cake
    41 AXValue:
    42 AXFocused: 0
    43 AXEnabled: 1
    44 AXWindow: <AXImage>
    45 AXSelectedTextMarkerRange: (null)
    46 AXStartTextMarker: <AXImage>
    47 AXEndTextMarker: <AXImage>
    48 AXVisited: 0
    49 AXLinkedUIElements: (null)
    50 AXSelected: 0
    51 AXBlockQuoteLevel: 0
    52 AXTopLevelUIElement: <AXImage>
    53 AXURL: LayoutTests/accessibility/resources/cake.png
    54 AXAccessKey: (null)
    55 
    56 
    57 

  • trunk/LayoutTests/platform/mac/accessibility/image-link-expected.txt

    r137484 r137512  
    99AXChildren: <array of size 1>
    1010AXHelp:
    11 AXParent: <AXLink>
     11AXParent: <AXLink: 'Delicious cake'>
    1212AXSize: NSSize: {280, 213}
    13 AXTitle:
     13AXTitle: Delicious cake
    1414AXDescription:
    1515AXValue:
    1616AXFocused: 1
    1717AXEnabled: 1
    18 AXWindow: <AXLink>
     18AXWindow: <AXLink: 'Delicious cake'>
    1919AXSelectedTextMarkerRange: (null)
    20 AXStartTextMarker: <AXLink>
    21 AXEndTextMarker: <AXLink>
     20AXStartTextMarker: <AXLink: 'Delicious cake'>
     21AXEndTextMarker: <AXLink: 'Delicious cake'>
    2222AXVisited: 0
    2323AXLinkedUIElements: (null)
    2424AXSelected: 0
    2525AXBlockQuoteLevel: 0
    26 AXTopLevelUIElement: <AXLink>
     26AXTopLevelUIElement: <AXLink: 'Delicious cake'>
    2727AXURL: http://www.wowhead.com/?item=33924
    2828AXAccessKey: (null)

  • trunk/LayoutTests/platform/mac/accessibility/internal-link-anchors2-expected.txt

    r137484 r137512  
    66AXChildren: <array of size 4>
    77AXHelp:
    8 AXParent: <AXHeading: '[edit] Tourette syndrome'>
     8AXParent: <AXHeading: '[] Tourette syndrome'>
    99AXSize: NSSize: {769, 22}
    10 AXTitle: [edit] Tourette syndrome
     10AXTitle: [] Tourette syndrome
    1111AXDescription:
    1212AXValue: 3
    1313AXFocused: 0
    1414AXEnabled: 1
    15 AXWindow: <AXHeading: '[edit] Tourette syndrome'>
     15AXWindow: <AXHeading: '[] Tourette syndrome'>
    1616AXSelectedTextMarkerRange: (null)
    17 AXStartTextMarker: <AXHeading: '[edit] Tourette syndrome'>
    18 AXEndTextMarker: <AXHeading: '[edit] Tourette syndrome'>
     17AXStartTextMarker: <AXHeading: '[] Tourette syndrome'>
     18AXEndTextMarker: <AXHeading: '[] Tourette syndrome'>
    1919AXVisited: 0
    2020AXLinkedUIElements: (null)

  • trunk/LayoutTests/platform/mac/accessibility/static-text-role-uses-text-under-element-expected.txt

    r62691 r137512  
    55
    66
    7 PASS text.stringValue is 'AXValue: Text 1 Text 2 Text 3'
     7PASS text.stringValue is 'AXValue:   Text 3'
    88PASS successfullyParsed is true
    99

  • trunk/LayoutTests/platform/mac/accessibility/static-text-role-uses-text-under-element.html

    r120111 r137512  
    2727        document.getElementById("text1").focus();
    2828        var text = accessibilityController.focusedElement;
    29         shouldBe("text.stringValue", "'AXValue: Text 1 Text 2 Text 3'");
     29        shouldBe("text.stringValue", "'AXValue:   Text 3'");
    3030    }
    3131

  • trunk/LayoutTests/platform/mac/accessibility/table-with-aria-role-expected.txt

    r137484 r137512  
    88AXChildren: <array of size 0>
    99AXHelp:
    10 AXParent: <AXButton: 'test      test    test
    11 test    test    test
    12 '>
     10AXParent: <AXButton: 'testtesttesttesttesttest'>
    1311AXSize: NSSize: {85, 52}
    14 AXTitle: test   test    test
    15 test    test    test
    16 
     12AXTitle: testtesttesttesttesttest
    1713AXDescription:
    1814AXFocused: 0
    1915AXEnabled: 1
    20 AXWindow: <AXButton: 'test      test    test
    21 test    test    test
    22 '>
     16AXWindow: <AXButton: 'testtesttesttesttesttest'>
    2317AXSelectedTextMarkerRange: (null)
    24 AXStartTextMarker: <AXButton: 'test     test    test
    25 test    test    test
    26 '>
    27 AXEndTextMarker: <AXButton: 'test       test    test
    28 test    test    test
    29 '>
     18AXStartTextMarker: <AXButton: 'testtesttesttesttesttest'>
     19AXEndTextMarker: <AXButton: 'testtesttesttesttesttest'>
    3020AXVisited: 0
    3121AXLinkedUIElements: (null)
    3222AXSelected: 0
    3323AXBlockQuoteLevel: 0
    34 AXTopLevelUIElement: <AXButton: 'test   test    test
    35 test    test    test
    36 '>
     24AXTopLevelUIElement: <AXButton: 'testtesttesttesttesttest'>
    3725AXTitleUIElement: (null)
    3826AXAccessKey: (null)

  • trunk/Source/WebCore/ChangeLog

    r137510 r137512  
     12012-12-12  Dominic Mazzoni  <dmazzoni@google.com>
     2
     3        AX: textUnderElement should consider alt text, but skip links and controls
     4        https://bugs.webkit.org/show_bug.cgi?id=101650
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Getting inner text from an element now ignores focusable descendants and
     9        containers, but uses alternative text.
     10
     11        The computation of textUnderElement is now recursive. This caused a crash
     12        if it was called by accessibilityIsIgnored during object destruction,
     13        so I simplified accessibilityIsIgnored to not actually call textUnderElement,
     14        without affecting test results, which should actually be a decent speedup.
     15
     16        Test: accessibility/button-title-uses-inner-img-alt.html
     17        Test: accessibility/focusable-div.html
     18
     19        * accessibility/AccessibilityNodeObject.cpp:
     20        (WebCore):
     21        (WebCore::shouldUseAccessiblityObjectInnerText):
     22        (WebCore::AccessibilityNodeObject::textUnderElement):
     23        * accessibility/AccessibilityRenderObject.cpp:
     24        (WebCore::AccessibilityRenderObject::textUnderElement):
     25
    1262012-12-12  Eberhard Graether  <egraether@google.com>
    227

  • trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp

    r137416 r137512  
    14351435}
    14361436
     1437// When building the textUnderElement for an object, determine whether or not
     1438// we should include the inner text of this given descendant object or skip it.
     1439static bool shouldUseAccessiblityObjectInnerText(AccessibilityObject* obj)
     1440{
     1441    // Consider this hypothetical example:
     1442    // <div tabindex=0>
     1443    //   <h2>
     1444    //     Table of contents
     1445    //   </h2>
     1446    //   <a href="#start">Jump to start of book</a>
     1447    //   <ul>
     1448    //     <li><a href="#1">Chapter 1</a></li>
     1449    //     <li><a href="#1">Chapter 2</a></li>
     1450    //   </ul>
     1451    // </div>
     1452    //
     1453    // The goal is to return a reasonable title for the outer container div, because
     1454    // it's focusable - but without making its title be the full inner text, which is
     1455    // quite long. As a heuristic, skip links, controls, and elements that are usually
     1456    // containers with lots of children.
     1457
     1458    // Skip focusable children, so we don't include the text of links and controls.
     1459    if (obj->canSetFocusAttribute())
     1460        return false;
     1461
     1462    // Skip big container elements like lists, tables, etc.
     1463    if (obj->isList() || obj->isAccessibilityTable() || obj->isTree() || obj->isCanvas())
     1464        return false;
     1465
     1466    return true;
     1467}
     1468
    14371469String AccessibilityNodeObject::textUnderElement() const
    14381470{
    14391471    Node* node = this->node();
    1440     if (!node)
    1441         return String();
    1442 
    1443     // Note: TextIterator doesn't return any text for nodes that don't have renderers.
    1444     // If this could be fixed, it'd be more accurate use TextIterator here.
    1445     if (node->isElementNode())
    1446         return toElement(node)->innerText();
    1447     else if (node->isTextNode())
     1472    if (node && node->isTextNode())
    14481473        return toText(node)->wholeText();
    1449    
    1450     return String();
     1474
     1475    String result;
     1476    for (AccessibilityObject* child = firstChild(); child; child = child->nextSibling()) {
     1477        if (!shouldUseAccessiblityObjectInnerText(child))
     1478            continue;
     1479
     1480        if (child->isAccessibilityNodeObject()) {
     1481            Vector<AccessibilityText> textOrder;
     1482            toAccessibilityNodeObject(child)->alternativeText(textOrder);
     1483            if (textOrder.size() > 0) {
     1484                result.append(textOrder[0].text);
     1485                continue;
     1486            }
     1487        }
     1488
     1489        result.append(child->textUnderElement());
     1490    }
     1491
     1492    return result;
    14511493}
    14521494

  • trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

    r137416 r137512  
    625625    if (!m_renderer)
    626626        return String();
    627    
     627
    628628    if (m_renderer->isFileUploadControl())
    629629        return toRenderFileUploadControl(m_renderer)->buttonValue();
    630630   
    631     Node* node = m_renderer->node();
    632 
    633631#if ENABLE(MATHML)
    634632    // Math operators create RenderText nodes on the fly that are not tied into the DOM in a reasonable way,
    … …  
    641639    }
    642640#endif
    643    
    644     if (node) {
    645         if (Frame* frame = node->document()->frame()) {
    646             // catch stale WebCoreAXObject (see <rdar://problem/3960196>)
    647             if (frame->document() != node->document())
    648                 return String();
    649 
    650             return plainText(rangeOfContents(node).get(), textIteratorBehaviorForTextRange());
     641
     642    if (m_renderer->isText()) {
     643        // If possible, use a text iterator to get the text, so that whitespace
     644        // is handled consistently.
     645        if (Node* node = this->node()) {
     646            if (Frame* frame = node->document()->frame()) {
     647                // catch stale WebCoreAXObject (see <rdar://problem/3960196>)
     648                if (frame->document() != node->document())
     649                    return String();
     650
     651                return plainText(rangeOfContents(node).get(), textIteratorBehaviorForTextRange());
     652            }
    651653        }
    652     }
    653    
    654     // Sometimes text fragments don't have Node's associated with them (like when
    655     // CSS content is used to insert text).
    656     if (m_renderer->isText()) {
     654   
     655        // Sometimes text fragments don't have Nodes associated with them (like when
     656        // CSS content is used to insert text).
    657657        RenderText* renderTextObject = toRenderText(m_renderer);
    658658        if (renderTextObject->isTextFragment())
    … …  
    660660    }
    661661   
    662     // return the null string for anonymous text because it is non-trivial to get
    663     // the actual text and, so far, that is not needed
    664     return String();
     662    return AccessibilityNodeObject::textUnderElement();
    665663}
    666664
Note: See TracChangeset for help on using the changeset viewer.