Changeset 158617 in webkit


Ignore:
Timestamp:
Nov 4, 2013, 4:47:32 PM (11 years ago)
Author:
Chris Fleizach
Message:

AX: Mail attachments at the start of an email are not output by VoiceOver
​https://bugs.webkit.org/show_bug.cgi?id=123697

Reviewed by Ryosuke Niwa.

Source/WebCore:

VoiceOver is expecting that "replaced elements" (attachments in a Mail message in this case) to be
represented by the replacement character when asking for a stringForRange.

However, when that replaced element is at the beginning of the document, the logic does not work because
there is a few code places that will take that first Position and advance it forward, not accounting for replaced elements.
When using the TextIterator normally, it does account for these, so that's why it's only affecting as at the beginning of the document.

Also a "replaced element" can be more than just renderer->isReplaced(), hence the externing of the isRendererReplacedElement method
and using that it in pertinent places.

Tests: platform/mac/accessibility/object-replacement-with-no-rendered-children-at-node-start.html

platform/mac/accessibility/object-replacement-with-rendered-children-at-node-start.html

  • accessibility/AccessibilityObject.cpp:

(WebCore::replacedNodeNeedsCharacter):

  • accessibility/mac/WebAccessibilityObjectWrapperMac.mm:

(nsStringForReplacedNode):

  • dom/Position.cpp:

(WebCore::Position::isCandidate):

  • dom/PositionIterator.cpp:
  • dom/Range.cpp:

(WebCore::Range::firstNode):

  • editing/TextIterator.cpp:

(WebCore::isRendererReplacedElement):

  • editing/TextIterator.h:

LayoutTests:

Add two flavors of this. One where the replaced element has rendered children and one where it does not,
which go down two different code paths.

  • platform/mac/accessibility/object-replacement-with-no-rendered-children-at-node-start-expected.txt: Added.
  • platform/mac/accessibility/object-replacement-with-no-rendered-children-at-node-start.html: Added.
  • platform/mac/accessibility/object-replacement-with-rendered-children-at-node-start-expected.txt: Added.
  • platform/mac/accessibility/object-replacement-with-rendered-children-at-node-start.html: Added.
Location:
trunk
Files:
4 added
17 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r158614 r158617  
     12013-11-04  Chris Fleizach  <cfleizach@apple.com>
     2
     3        AX: Mail attachments at the start of an email are not output by VoiceOver
     4        https://bugs.webkit.org/show_bug.cgi?id=123697
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Add two flavors of this. One where the replaced element has rendered children and one where it does not,
     9        which go down two different code paths.
     10
     11        * platform/mac/accessibility/object-replacement-with-no-rendered-children-at-node-start-expected.txt: Added.
     12        * platform/mac/accessibility/object-replacement-with-no-rendered-children-at-node-start.html: Added.
     13        * platform/mac/accessibility/object-replacement-with-rendered-children-at-node-start-expected.txt: Added.
     14        * platform/mac/accessibility/object-replacement-with-rendered-children-at-node-start.html: Added.
     15
    1162013-11-04  Ryosuke Niwa  <rniwa@webkit.org>
    217

  • trunk/Source/WebCore/ChangeLog

    r158611 r158617  
     12013-11-04  Chris Fleizach  <cfleizach@apple.com>
     2
     3        AX: Mail attachments at the start of an email are not output by VoiceOver
     4        https://bugs.webkit.org/show_bug.cgi?id=123697
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        VoiceOver is expecting that "replaced elements" (attachments in a Mail message in this case) to be
     9        represented by the replacement character when asking for a stringForRange.
     10
     11        However, when that replaced element is at the beginning of the document, the logic does not work because
     12        there is a few code places that will take that first Position and advance it forward, not accounting for replaced elements.
     13        When using the TextIterator normally, it does account for these, so that's why it's only affecting as at the beginning of the document.
     14
     15        Also a "replaced element" can be more than just renderer->isReplaced(), hence the externing of the isRendererReplacedElement method
     16        and using that it in pertinent places.
     17
     18        Tests: platform/mac/accessibility/object-replacement-with-no-rendered-children-at-node-start.html
     19               platform/mac/accessibility/object-replacement-with-rendered-children-at-node-start.html
     20
     21        * accessibility/AccessibilityObject.cpp:
     22        (WebCore::replacedNodeNeedsCharacter):
     23        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
     24        (nsStringForReplacedNode):
     25        * dom/Position.cpp:
     26        (WebCore::Position::isCandidate):
     27        * dom/PositionIterator.cpp:
     28        * dom/Range.cpp:
     29        (WebCore::Range::firstNode):
     30        * editing/TextIterator.cpp:
     31        (WebCore::isRendererReplacedElement):
     32        * editing/TextIterator.h:
     33
    1342013-11-04  Andreas Kling  <akling@apple.com>
    235

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

    r158163 r158617  
    840840    // we should always be given a rendered node and a replaced node, but be safe
    841841    // replaced nodes are either attachments (widgets) or images
    842     if (!replacedNode || !replacedNode->renderer() || !replacedNode->renderer()->isReplaced() || replacedNode->isTextNode())
     842    if (!replacedNode || !isRendererReplacedElement(replacedNode->renderer()) || replacedNode->isTextNode())
    843843        return false;
    844844

  • trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm

    r158332 r158617  
    874874    // we should always be given a rendered node and a replaced node, but be safe
    875875    // replaced nodes are either attachments (widgets) or images
    876     if (!replacedNode || !replacedNode->renderer() || !replacedNode->renderer()->isReplaced() || replacedNode->isTextNode()) {
     876    if (!replacedNode || !isRendererReplacedElement(replacedNode->renderer()) || replacedNode->isTextNode()) {
    877877        ASSERT_NOT_REACHED();
    878878        return nil;

  • trunk/Source/WebCore/dom/Position.cpp

    r158163 r158617  
    938938        return false;
    939939       
     940    if (isRendererReplacedElement(renderer))
     941        return !nodeIsUserSelectNone(deprecatedNode()) && atFirstEditingPositionForNode();
     942
    940943    if (renderer->isRenderBlockFlow()) {
    941944        RenderBlock& block = toRenderBlock(*renderer);

  • trunk/Source/WebCore/dom/PositionIterator.cpp

    r158569 r158617  
    3030#include "RenderBlock.h"
    3131#include "RenderText.h"
     32#include "TextIterator.h"
    3233#include "htmlediting.h"
    3334

  • trunk/Source/WebCore/dom/Range.cpp

    r158569 r158617  
    15511551    if (m_start.container()->offsetInCharacters())
    15521552        return m_start.container();
     1553    if (isRendererReplacedElement(m_start.container()->renderer()))
     1554        return m_start.container();
    15531555    if (Node* child = m_start.container()->childNode(m_start.offset()))
    15541556        return child;

  • trunk/Source/WebCore/editing/TextIterator.cpp

    r158551 r158617  
    246246}
    247247   
    248 static bool isRendererReplacedElement(RenderObject* renderer)
     248bool isRendererReplacedElement(RenderObject* renderer)
    249249{
    250250    if (!renderer)

  • trunk/Source/WebCore/editing/TextIterator.h

    r158428 r158617  
    6464String plainText(const Range*, TextIteratorBehavior defaultBehavior = TextIteratorDefaultBehavior, bool isDisplayString = false);
    6565PassRefPtr<Range> findPlainText(const Range*, const String&, FindOptions);
     66bool isRendererReplacedElement(RenderObject*);
    6667
    6768class BitStack {

  • trunk/Tools/DumpRenderTree/AccessibilityUIElement.cpp

    r157821 r158617  
    748748}
    749749
     750static JSValueRef startTextMarkerCallback(JSContextRef context, JSObjectRef thisObject, JSStringRef, JSValueRef*)
     751{
     752    return AccessibilityTextMarker::makeJSAccessibilityTextMarker(context, toAXElement(thisObject)->startTextMarker());
     753}
     754
     755static JSValueRef endTextMarkerCallback(JSContextRef context, JSObjectRef thisObject, JSStringRef, JSValueRef*)
     756{
     757    return AccessibilityTextMarker::makeJSAccessibilityTextMarker(context, toAXElement(thisObject)->endTextMarker());
     758}
     759
    750760// Static Value Getters
    751761
    … …  
    12391249
    12401250AccessibilityTextMarker AccessibilityUIElement::textMarkerForIndex(int)
     1251{
     1252    return 0;
     1253}
     1254
     1255AccessibilityTextMarker AccessibilityUIElement::startTextMarker()
     1256{
     1257    return 0;
     1258}
     1259
     1260AccessibilityTextMarker AccessibilityUIElement::endTextMarker()
    12411261{
    12421262    return 0;
    … …  
    13151335        { "horizontalScrollbar", horizontalScrollbarCallback, 0, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
    13161336        { "verticalScrollbar", verticalScrollbarCallback, 0, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
     1337        { "startTextMarker", startTextMarkerCallback, 0, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
     1338        { "endTextMarker", endTextMarkerCallback, 0, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
    13171339#if PLATFORM(IOS)
    13181340        { "iphoneLabel", getIPhoneLabelCallback, 0, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },

  • trunk/Tools/DumpRenderTree/AccessibilityUIElement.h

    r157821 r158617  
    230230    AccessibilityTextMarker nextTextMarker(AccessibilityTextMarker*);
    231231    AccessibilityUIElement accessibilityElementForTextMarker(AccessibilityTextMarker*);
     232    AccessibilityTextMarker startTextMarker();
     233    AccessibilityTextMarker endTextMarker();
     234   
    232235    JSStringRef stringForTextMarkerRange(AccessibilityTextMarkerRange*);
    233236    int textMarkerRangeLength(AccessibilityTextMarkerRange*);

  • trunk/Tools/DumpRenderTree/mac/AccessibilityUIElementMac.mm

    r157821 r158617  
    15641564}
    15651565
     1566AccessibilityTextMarker AccessibilityUIElement::startTextMarker()
     1567{
     1568    BEGIN_AX_OBJC_EXCEPTIONS
     1569    id textMarker = [m_element accessibilityAttributeValue:@"AXStartTextMarker"];
     1570    return AccessibilityTextMarker(textMarker);
     1571    END_AX_OBJC_EXCEPTIONS
     1572   
     1573    return 0;
     1574}
     1575
     1576AccessibilityTextMarker AccessibilityUIElement::endTextMarker()
     1577{
     1578    BEGIN_AX_OBJC_EXCEPTIONS
     1579    id textMarker = [m_element accessibilityAttributeValue:@"AXEndTextMarker"];
     1580    return AccessibilityTextMarker(textMarker);
     1581    END_AX_OBJC_EXCEPTIONS
     1582   
     1583    return 0;
     1584}
     1585
    15661586#endif // SUPPORTS_AX_TEXTMARKERS
    15671587

  • trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp

    r157821 r158617  
    178178PassRefPtr<AccessibilityTextMarker> AccessibilityUIElement::previousTextMarker(AccessibilityTextMarker*) { return 0; }
    179179PassRefPtr<AccessibilityTextMarker> AccessibilityUIElement::nextTextMarker(AccessibilityTextMarker*) { return 0; }
     180PassRefPtr<AccessibilityTextMarker> AccessibilityUIElement::startTextMarker() { return 0; }
     181PassRefPtr<AccessibilityTextMarker> AccessibilityUIElement::endTextMarker() { return 0; }
    180182JSRetainPtr<JSStringRef> AccessibilityUIElement::stringForTextMarkerRange(AccessibilityTextMarkerRange*) { return 0; }
    181183bool AccessibilityUIElement::attributedStringForTextMarkerRangeContainsAttribute(JSStringRef, AccessibilityTextMarkerRange*) { return false; }

  • trunk/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h

    r158266 r158617  
    226226    bool isTextMarkerValid(AccessibilityTextMarker*);
    227227    PassRefPtr<AccessibilityTextMarker> textMarkerForIndex(int);
     228    PassRefPtr<AccessibilityTextMarker> startTextMarker();
     229    PassRefPtr<AccessibilityTextMarker> endTextMarker();
    228230
    229231    // Returns an ordered list of supported actions for an element.

  • trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl

    r157821 r158617  
    170170    boolean isTextMarkerValid(AccessibilityTextMarker marker);
    171171    AccessibilityTextMarker textMarkerForIndex(int textIndex);
     172    readonly attribute AccessibilityTextMarker startTextMarker;
     173    readonly attribute AccessibilityTextMarker endTextMarker;
    172174
    173175    // Returns an ordered list of supported actions for an element.

  • trunk/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp

    r158576 r158617  
    14691469    return 0;
    14701470}
     1471   
     1472PassRefPtr<AccessibilityTextMarker> AccessibilityUIElement::startTextMarker()
     1473{
     1474    // FIXME: implement
     1475    return 0;   
     1476}
     1477
     1478PassRefPtr<AccessibilityTextMarker> AccessibilityUIElement::endTextMarker()
     1479{
     1480    // FIXME: implement
     1481    return 0;
     1482}
    14711483
    14721484void AccessibilityUIElement::scrollToMakeVisible()

  • trunk/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm

    r157821 r158617  
    15431543    return 0;                                                                         
    15441544}
     1545   
     1546PassRefPtr<AccessibilityTextMarker> AccessibilityUIElement::startTextMarker()
     1547{
     1548    BEGIN_AX_OBJC_EXCEPTIONS
     1549    id textMarker = [m_element accessibilityAttributeValue:@"AXStartTextMarker"];
     1550    return AccessibilityTextMarker::create(textMarker);
     1551    END_AX_OBJC_EXCEPTIONS
     1552   
     1553    return 0;
     1554}
     1555
     1556PassRefPtr<AccessibilityTextMarker> AccessibilityUIElement::endTextMarker()
     1557{
     1558    BEGIN_AX_OBJC_EXCEPTIONS
     1559    id textMarker = [m_element accessibilityAttributeValue:@"AXEndTextMarker"];
     1560    return AccessibilityTextMarker::create(textMarker);
     1561    END_AX_OBJC_EXCEPTIONS
     1562   
     1563    return 0;
     1564}
    15451565
    15461566static NSString *_convertMathMultiscriptPairsToString(NSArray *pairs)
Note: See TracChangeset for help on using the changeset viewer.