Changeset 158428 in webkit


Ignore:
Timestamp:
Nov 1, 2013 3:59:33 AM (10 years ago)
Author:
mario@webkit.org
Message:

[ATK] Avoid explicit traversal of text controls and the render tree in AtkText implementation
​https://bugs.webkit.org/show_bug.cgi?id=123153

Reviewed by Chris Fleizach.

Source/WebCore:

Remove functions from the AtkText implementation that manually
walk the render tree to compose the text for a exposed objects in
certain cases (e.g. anonymous blocks, text controls).

The reason for this change is that the current implementation
follows an error-prone approach, since by doing things like
manually walking the render tree from here we are not properly
considering all the possible scenarios that might happen when
traversing text. This, however, is a task that is better suited
for the TextIterator, which is already written to consider all
those cases and to emit the proper character in every single
situation: text nodes, replaced objects and so on.

So, by removing all that too specific code (textForObject() and
textForRenderer() mainly) from WebKitAccessibleInterfaceText.cpp
and relying in AccessibilityObject::textUnderElement(), which it
ends up using the TextIterator for certain cases, we have a much
better and robust method of retrieving the text associated with an
instance of AtkObject implementing the AtkText interface.

  • accessibility/atk/WebKitAccessibleInterfaceText.cpp:

(webkitAccessibleTextGetText): Removed call to textForObject(), now that
we have just removed that function, together with textForRenderer().

Make AccessibilityRenderObject::textUnderElement() able to deal with
anonymous blocks directly, by creating a range based in the boundaries
defined by the first and last child renderers for that block. This will
make possible to treat an anonymous block as a whole instead of having
to rely in the concatenation of each of its children, as it does now.

  • accessibility/AccessibilityRenderObject.cpp:

(WebCore::AccessibilityRenderObject::textUnderElement): Added a new code
path to deal with anonymous blocks for text renderers, or when including
all the children is explicitly requested.

Modified TextIterator so text for children of replaced objects are
ignored if we are emmiting the special character for those objects.

  • editing/TextIterator.cpp:

(WebCore::TextIterator::handleReplacedElement): Make sure no children are
handled a replaced object if m_emitsObjectReplacementCharacters is set.

  • editing/TextIterator.h: Updated m_emitsObjectReplacementCharacters

description to reflect the new behavior.

LayoutTests:

Updated test expectations to properly reflect the new reality when it
comes to exposing replaced objects and anymous blocks.

  • platform/gtk/accessibility/table-with-rules-expected.txt: Updated to

print <\n> explicitly for the two instances of <BR> that are present in the
test, that will be included as part of an anonymous block.

  • platform/efl/accessibility/table-with-rules-expected.txt: Ditto.
  • platform/efl-wk2/accessibility/table-with-rules-expected.txt: Ditto.
  • platform/efl/accessibility/deleting-iframe-destroys-axcache-expected.txt: Updated

expectations not to expect the text of a button to be shown.

  • platform/gtk/accessibility/deleting-iframe-destroys-axcache-expected.txt: Ditto.
  • platform/gtk/accessibility/replaced-objects-in-anonymous-blocks-expected.txt: Ditto.
  • platform/gtk/TestExpectations: Removed replaced-objects-in-anonymous-blocks.html

from the list of expected failures, as it's now being properly exposed.

Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r158427 r158428  
     12013-11-01  Mario Sanchez Prada  <mario.prada@samsung.com>
     2
     3        [ATK] Avoid explicit traversal of text controls and the render tree in AtkText implementation
     4        https://bugs.webkit.org/show_bug.cgi?id=123153
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Updated test expectations to properly reflect the new reality when it
     9        comes to exposing replaced objects and anymous blocks.
     10
     11        * platform/gtk/accessibility/table-with-rules-expected.txt: Updated to
     12        print <\n> explicitly for the two instances of <BR> that are present in the
     13        test, that will be included as part of an anonymous block.
     14        * platform/efl/accessibility/table-with-rules-expected.txt: Ditto.
     15        * platform/efl-wk2/accessibility/table-with-rules-expected.txt: Ditto.
     16
     17        * platform/efl/accessibility/deleting-iframe-destroys-axcache-expected.txt: Updated
     18        expectations not to expect the text of a button to be shown.
     19        * platform/gtk/accessibility/deleting-iframe-destroys-axcache-expected.txt: Ditto.
     20        * platform/gtk/accessibility/replaced-objects-in-anonymous-blocks-expected.txt: Ditto.
     21
     22        * platform/gtk/TestExpectations: Removed replaced-objects-in-anonymous-blocks.html
     23        from the list of expected failures, as it's now being properly exposed.
     24
    1252013-11-01  Alexey Proskuryakov  <ap@apple.com>
    226

  • trunk/LayoutTests/platform/efl-wk2/accessibility/table-with-rules-expected.txt

    r157564 r158428  
    8585AXTitle:
    8686AXDescription:
    87 AXValue: ------------------------------------
     87AXValue: <\n>------------------------------------<\n>
    8888AXFocusable: 0
    8989AXFocused: 0

  • trunk/LayoutTests/platform/efl/accessibility/deleting-iframe-destroys-axcache-expected.txt

    r157394 r158428  
    1616        AXRole: AXParagraph AXValue: Before
    1717        AXRole: AXGroup AXValue: <obj>
    18             AXRole: AXScrollArea
    19                 AXRole: AXWebArea
    20                     AXRole: AXGroup AXValue: <obj>Click me
    21                         AXRole: AXButton
     18            AXRole: AXGroup
     19                AXRole: AXScrollArea
     20                    AXRole: AXWebArea
     21                        AXRole: AXGroup AXValue: <obj>
     22                            AXRole: AXButton
    2223        AXRole: AXParagraph AXValue: After
    2324        AXRole: AXParagraph AXValue: End of test

  • trunk/LayoutTests/platform/efl/accessibility/table-with-rules-expected.txt

    r157564 r158428  
    8585AXTitle:
    8686AXDescription:
    87 AXValue: ------------------------------------
     87AXValue: <\n>------------------------------------<\n>
    8888AXFocusable: 0
    8989AXFocused: 0

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r158396 r158428  
    13351335webkit.org/b/114259 platform/gtk/accessibility/combo-box-collapsed-selection-changed.html [ Failure ]
    13361336
    1337 webkit.org/b/118577 platform/gtk/accessibility/replaced-objects-in-anonymous-blocks.html [ Failure ]
    1338 
    13391337webkit.org/b/114612 editing/style/block-style-005.html [ Failure ]
    13401338

  • trunk/LayoutTests/platform/gtk/accessibility/deleting-iframe-destroys-axcache-expected.txt

    r156532 r158428  
    1616        AXRole: AXParagraph AXValue: Before
    1717        AXRole: AXGroup AXValue: <obj>
    18             AXRole: AXScrollArea
    19                 AXRole: AXWebArea
    20                     AXRole: AXGroup AXValue: <obj>Click me
    21                         AXRole: AXButton
     18            AXRole: AXGroup
     19                AXRole: AXScrollArea
     20                    AXRole: AXWebArea
     21                        AXRole: AXGroup AXValue: <obj>
     22                            AXRole: AXButton
    2223        AXRole: AXParagraph AXValue: After
    2324        AXRole: AXParagraph AXValue: End of test

  • trunk/LayoutTests/platform/gtk/accessibility/replaced-objects-in-anonymous-blocks-expected.txt

    r131674 r158428  
    1313AXRole: AXWebArea
    1414    AXRole: AXParagraph AXValue: Paragraph
    15     AXRole: AXGroup AXValue: <obj>button<\n>
     15    AXRole: AXGroup AXValue: <obj>
    1616        AXRole: AXButton
    1717    AXRole: AXGroup AXValue: <obj>
    18         AXRole: AXScrollArea
    19             AXRole: AXWebArea
    20     AXRole: AXGroup AXValue: <obj>text area<\n>
     18        AXRole: AXGroup
     19            AXRole: AXScrollArea
     20                AXRole: AXWebArea
     21    AXRole: AXGroup AXValue: <obj>
    2122        AXRole: AXTextField AXValue: text area
    22     AXRole: AXDiv AXValue: Div<\n><obj>button
     23    AXRole: AXDiv AXValue: Div<\n><obj>
    2324        AXRole: AXButton
    24     AXRole: AXHeading AXValue: Heading<\n><obj>button<\n><obj>
     25    AXRole: AXHeading AXValue: Heading<\n><obj><\n><obj>
    2526        AXRole: AXButton
    2627        AXRole: AXImage

  • trunk/LayoutTests/platform/gtk/accessibility/table-with-rules-expected.txt

    r156033 r158428  
    8585AXTitle:
    8686AXDescription:
    87 AXValue: ------------------------------------
     87AXValue: <\n>------------------------------------<\n>
    8888AXFocusable: 0
    8989AXFocused: 0

  • trunk/Source/WebCore/ChangeLog

    r158427 r158428  
     12013-11-01  Mario Sanchez Prada  <mario.prada@samsung.com>
     2
     3        [ATK] Avoid explicit traversal of text controls and the render tree in AtkText implementation
     4        https://bugs.webkit.org/show_bug.cgi?id=123153
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Remove functions from the AtkText implementation that manually
     9        walk the render tree to compose the text for a exposed objects in
     10        certain cases (e.g. anonymous blocks, text controls).
     11
     12        The reason for this change is that the current implementation
     13        follows an error-prone approach, since by doing things like
     14        manually walking the render tree from here we are not properly
     15        considering all the possible scenarios that might happen when
     16        traversing text. This, however, is a task that is better suited
     17        for the TextIterator, which is already written to consider all
     18        those cases and to emit the proper character in every single
     19        situation: text nodes, replaced objects and so on.
     20
     21        So, by removing all that too specific code (textForObject() and
     22        textForRenderer() mainly) from WebKitAccessibleInterfaceText.cpp
     23        and relying in AccessibilityObject::textUnderElement(), which it
     24        ends up using the TextIterator for certain cases, we have a much
     25        better and robust method of retrieving the text associated with an
     26        instance of AtkObject implementing the AtkText interface.
     27
     28        * accessibility/atk/WebKitAccessibleInterfaceText.cpp:
     29        (webkitAccessibleTextGetText): Removed call to textForObject(), now that
     30        we have just removed that function, together with textForRenderer().
     31
     32        Make AccessibilityRenderObject::textUnderElement() able to deal with
     33        anonymous blocks directly, by creating a range based in the boundaries
     34        defined by the first and last child renderers for that block. This will
     35        make possible to treat an anonymous block as a whole instead of having
     36        to rely in the concatenation of each of its children, as it does now.
     37
     38        * accessibility/AccessibilityRenderObject.cpp:
     39        (WebCore::AccessibilityRenderObject::textUnderElement): Added a new code
     40        path to deal with anonymous blocks for text renderers, or when including
     41        all the children is explicitly requested.
     42
     43        Modified TextIterator so text for children of replaced objects are
     44        ignored if we are emmiting the special character for those objects.
     45
     46        * editing/TextIterator.cpp:
     47        (WebCore::TextIterator::handleReplacedElement): Make sure no children are
     48        handled a replaced object if m_emitsObjectReplacementCharacters is set.
     49        * editing/TextIterator.h: Updated m_emitsObjectReplacementCharacters
     50        description to reflect the new behavior.
     51
    1522013-11-01  Alexey Proskuryakov  <ap@apple.com>
    253

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

    r158195 r158428  
    653653        // If possible, use a text iterator to get the text, so that whitespace
    654654        // is handled consistently.
    655         if (Node* node = this->node()) {
    656             if (Frame* frame = node->document().frame()) {
     655        Document* nodeDocument = 0;
     656        RefPtr<Range> textRange;
     657        if (Node* node = m_renderer->node()) {
     658            nodeDocument = &node->document();
     659            textRange = rangeOfContents(*node);
     660        } else {
     661            // For anonymous blocks, we work around not having a direct node to create a range from
     662            // defining one based in the two external positions defining the boundaries of the subtree.
     663            RenderObject* firstChildRenderer = m_renderer->firstChildSlow();
     664            RenderObject* lastChildRenderer = m_renderer->lastChildSlow();
     665            if (firstChildRenderer && lastChildRenderer) {
     666                ASSERT(firstChildRenderer->node());
     667                ASSERT(lastChildRenderer->node());
     668
     669                // We define the start and end positions for the range as the ones right before and after
     670                // the first and the last nodes in the DOM tree that is wrapped inside the anonymous block.
     671                Node* firstNodeInBlock = firstChildRenderer->node();
     672                Position startPosition = positionInParentBeforeNode(firstNodeInBlock);
     673                Position endPosition = positionInParentAfterNode(lastChildRenderer->node());
     674
     675                nodeDocument = &firstNodeInBlock->document();
     676                textRange = Range::create(*nodeDocument, startPosition, endPosition);
     677            }
     678        }
     679
     680        if (nodeDocument && textRange) {
     681            if (Frame* frame = nodeDocument->frame()) {
    657682                // catch stale WebCoreAXObject (see <rdar://problem/3960196>)
    658                 if (frame->document() != &node->document())
     683                if (frame->document() != nodeDocument)
    659684                    return String();
    660 
    661                 return plainText(rangeOfContents(*node).get(), textIteratorBehaviorForTextRange());
     685                return plainText(textRange.get(), textIteratorBehaviorForTextRange());
    662686            }
    663687        }

  • trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp

    r158352 r158428  
    7070}
    7171
    72 static gchar* textForRenderer(RenderObject* renderer)
    73 {
    74     GString* resultText = g_string_new(0);
    75 
    76     if (!renderer)
    77         return g_string_free(resultText, FALSE);
    78 
    79     // For RenderBlocks, piece together the text from the RenderText objects they contain.
    80     for (RenderObject* object = renderer->firstChildSlow(); object; object = object->nextSibling()) {
    81         if (object->isBR()) {
    82             g_string_append(resultText, "\n");
    83             continue;
    84         }
    85 
    86         RenderText* renderText;
    87         if (object->isText())
    88             renderText = toRenderText(object);
    89         else {
    90             // List item's markers will be treated in an special way
    91             // later on this function, so ignore them here.
    92             if (object->isReplaced() && !object->isListMarker())
    93                 g_string_append_unichar(resultText, objectReplacementCharacter);
    94 
    95             // We need to check children, if any, to consider when
    96             // current object is not a text object but some of its
    97             // children are, in order not to miss those portions of
    98             // text by not properly handling those situations
    99             if (object->firstChildSlow()) {
    100                 GOwnPtr<char> objectText(textForRenderer(object));
    101                 g_string_append(resultText, objectText.get());
    102             }
    103             continue;
    104         }
    105 
    106         InlineTextBox* box = renderText ? renderText->firstTextBox() : 0;
    107         while (box) {
    108             // WebCore introduces line breaks in the text that do not reflect
    109             // the layout you see on the screen, replace them with spaces.
    110             String text = String(renderText->characters(), renderText->textLength()).replace("\n", " ");
    111             g_string_append(resultText, text.substring(box->start(), box->end() - box->start() + 1).utf8().data());
    112 
    113             // Newline chars in the source result in separate text boxes, so check
    114             // before adding a newline in the layout. See bug 25415 comment #78.
    115             // If the next sibling is a BR, we'll add the newline when we examine that child.
    116             if (!box->nextOnLineExists() && !(object->nextSibling() && object->nextSibling()->isBR())) {
    117                 // If there was a '\n' in the last position of the
    118                 // current text box, it would have been converted to a
    119                 // space in String::replace(), so remove it first.
    120                 if (renderText->characters()[box->end()] == '\n')
    121                     g_string_erase(resultText, resultText->len - 1, -1);
    122 
    123                 g_string_append(resultText, "\n");
    124             }
    125             box = box->nextTextBox();
    126         }
    127     }
    128 
    129     // Insert the text of the marker for list item in the right place, if present
    130     if (renderer->isListItem()) {
    131         String markerText = toRenderListItem(renderer)->markerTextWithSuffix();
    132         if (renderer->style().direction() == LTR)
    133             g_string_prepend(resultText, markerText.utf8().data());
    134         else
    135             g_string_append(resultText, markerText.utf8().data());
    136     }
    137 
    138     return g_string_free(resultText, FALSE);
    139 }
    140 
    141 static gchar* textForObject(const AccessibilityObject* coreObject)
    142 {
    143     GString* str = g_string_new(0);
    144 
    145     // For text controls, we can get the text line by line.
    146     if (coreObject->isTextControl()) {
    147         unsigned textLength = coreObject->textLength();
    148         int lineNumber = 0;
    149         PlainTextRange range = coreObject->doAXRangeForLine(lineNumber);
    150         while (range.length) {
    151             String lineText = coreObject->doAXStringForRange(range);
    152             g_string_append(str, lineText.utf8().data());
    153 
    154             // When a line of text wraps in a text area, the final space
    155             // after each non-final line must be replaced with a line break.
    156             if (range.start + range.length < textLength)
    157                 g_string_overwrite_len(str, str->len - 1, "\n", 1);
    158 
    159             range = coreObject->doAXRangeForLine(++lineNumber);
    160         }
    161     } else if (coreObject->isAccessibilityRenderObject()) {
    162         GOwnPtr<gchar> rendererText(textForRenderer(coreObject->renderer()));
    163         g_string_append(str, rendererText.get());
    164     }
    165 
    166     return g_string_free(str, FALSE);
    167 }
    168 
    16972static int baselinePositionForRenderObject(RenderObject* renderObject)
    17073{
    … …  
    589492    }
    590493
    591     if (!ret.length()) {
    592         // This can happen at least with anonymous RenderBlocks (e.g. body text amongst paragraphs)
    593         // In such instances, there may also be embedded objects. The object replacement character
    594         // is something ATs want included and we have to account for the fact that it is multibyte.
    595         GOwnPtr<char> objectText(textForObject(coreObject));
    596         ret = String::fromUTF8(objectText.get());
    597     }
    598 
    599494    // Prefix a item number/bullet if needed
    600495    int actualEndOffset = endOffset == -1 ? ret.length() : endOffset;

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

    r158350 r158428  
    723723    if (m_emitsObjectReplacementCharacters && renderer && renderer->isReplaced()) {
    724724        emitCharacter(objectReplacementCharacter, m_node->parentNode(), m_node, 0, 1);
     725        // Don't process subtrees for embedded objects. If the text there is required,
     726        // it must be explicitly asked by specifying a range falling inside its boundaries.
     727        m_handledChildren = true;
    725728        return true;
    726729    }

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

    r146835 r158428  
    188188    // Used when the visibility of the style should not affect text gathering.
    189189    bool m_ignoresStyleVisibility;
    190     // Used when emitting the special 0xFFFC character is required.
     190    // Used when emitting the special 0xFFFC character is required. Children for replaced objects will be ignored.
    191191    bool m_emitsObjectReplacementCharacters;
    192192    // Used when the iteration should stop if form controls are reached.
Note: See TracChangeset for help on using the changeset viewer.