- Revision
- 158428
- Author
- [email protected]
- Date
- 2013-11-01 03:59:33 -0700 (Fri, 01 Nov 2013)
Log 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.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (158427 => 158428)
--- trunk/LayoutTests/ChangeLog 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/LayoutTests/ChangeLog 2013-11-01 10:59:33 UTC (rev 158428)
@@ -1,3 +1,27 @@
+2013-11-01 Mario Sanchez Prada <[email protected]>
+
+ [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.
+
+ 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.
+
2013-11-01 Alexey Proskuryakov <[email protected]>
Add a Mac WebCrypto implementation of HMAC importKey/sign/verify
Modified: trunk/LayoutTests/platform/efl/accessibility/deleting-iframe-destroys-axcache-expected.txt (158427 => 158428)
--- trunk/LayoutTests/platform/efl/accessibility/deleting-iframe-destroys-axcache-expected.txt 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/LayoutTests/platform/efl/accessibility/deleting-iframe-destroys-axcache-expected.txt 2013-11-01 10:59:33 UTC (rev 158428)
@@ -15,10 +15,11 @@
AXRole: AXWebArea
AXRole: AXParagraph AXValue: Before
AXRole: AXGroup AXValue: <obj>
- AXRole: AXScrollArea
- AXRole: AXWebArea
- AXRole: AXGroup AXValue: <obj>Click me
- AXRole: AXButton
+ AXRole: AXGroup
+ AXRole: AXScrollArea
+ AXRole: AXWebArea
+ AXRole: AXGroup AXValue: <obj>
+ AXRole: AXButton
AXRole: AXParagraph AXValue: After
AXRole: AXParagraph AXValue: End of test
Modified: trunk/LayoutTests/platform/efl/accessibility/table-with-rules-expected.txt (158427 => 158428)
--- trunk/LayoutTests/platform/efl/accessibility/table-with-rules-expected.txt 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/LayoutTests/platform/efl/accessibility/table-with-rules-expected.txt 2013-11-01 10:59:33 UTC (rev 158428)
@@ -84,7 +84,7 @@
AXSize: { 765.000000, 36.000000 }
AXTitle:
AXDescription:
-AXValue: ------------------------------------
+AXValue: <\n>------------------------------------<\n>
AXFocusable: 0
AXFocused: 0
AXSelectable: 0
Modified: trunk/LayoutTests/platform/efl-wk2/accessibility/table-with-rules-expected.txt (158427 => 158428)
--- trunk/LayoutTests/platform/efl-wk2/accessibility/table-with-rules-expected.txt 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/LayoutTests/platform/efl-wk2/accessibility/table-with-rules-expected.txt 2013-11-01 10:59:33 UTC (rev 158428)
@@ -84,7 +84,7 @@
AXSize: { 769.000000, 36.000000 }
AXTitle:
AXDescription:
-AXValue: ------------------------------------
+AXValue: <\n>------------------------------------<\n>
AXFocusable: 0
AXFocused: 0
AXSelectable: 0
Modified: trunk/LayoutTests/platform/gtk/TestExpectations (158427 => 158428)
--- trunk/LayoutTests/platform/gtk/TestExpectations 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2013-11-01 10:59:33 UTC (rev 158428)
@@ -1334,8 +1334,6 @@
webkit.org/b/114259 platform/gtk/accessibility/aria-slider-required-attributes.html [ Failure ]
webkit.org/b/114259 platform/gtk/accessibility/combo-box-collapsed-selection-changed.html [ Failure ]
-webkit.org/b/118577 platform/gtk/accessibility/replaced-objects-in-anonymous-blocks.html [ Failure ]
-
webkit.org/b/114612 editing/style/block-style-005.html [ Failure ]
webkit.org/b/115025 fast/events/constructors/mouse-event-constructor.html [ Failure ]
Modified: trunk/LayoutTests/platform/gtk/accessibility/deleting-iframe-destroys-axcache-expected.txt (158427 => 158428)
--- trunk/LayoutTests/platform/gtk/accessibility/deleting-iframe-destroys-axcache-expected.txt 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/LayoutTests/platform/gtk/accessibility/deleting-iframe-destroys-axcache-expected.txt 2013-11-01 10:59:33 UTC (rev 158428)
@@ -15,10 +15,11 @@
AXRole: AXWebArea
AXRole: AXParagraph AXValue: Before
AXRole: AXGroup AXValue: <obj>
- AXRole: AXScrollArea
- AXRole: AXWebArea
- AXRole: AXGroup AXValue: <obj>Click me
- AXRole: AXButton
+ AXRole: AXGroup
+ AXRole: AXScrollArea
+ AXRole: AXWebArea
+ AXRole: AXGroup AXValue: <obj>
+ AXRole: AXButton
AXRole: AXParagraph AXValue: After
AXRole: AXParagraph AXValue: End of test
Modified: trunk/LayoutTests/platform/gtk/accessibility/replaced-objects-in-anonymous-blocks-expected.txt (158427 => 158428)
--- trunk/LayoutTests/platform/gtk/accessibility/replaced-objects-in-anonymous-blocks-expected.txt 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/LayoutTests/platform/gtk/accessibility/replaced-objects-in-anonymous-blocks-expected.txt 2013-11-01 10:59:33 UTC (rev 158428)
@@ -12,16 +12,17 @@
End of test
AXRole: AXWebArea
AXRole: AXParagraph AXValue: Paragraph
- AXRole: AXGroup AXValue: <obj>button<\n>
+ AXRole: AXGroup AXValue: <obj>
AXRole: AXButton
AXRole: AXGroup AXValue: <obj>
- AXRole: AXScrollArea
- AXRole: AXWebArea
- AXRole: AXGroup AXValue: <obj>text area<\n>
+ AXRole: AXGroup
+ AXRole: AXScrollArea
+ AXRole: AXWebArea
+ AXRole: AXGroup AXValue: <obj>
AXRole: AXTextField AXValue: text area
- AXRole: AXDiv AXValue: Div<\n><obj>button
+ AXRole: AXDiv AXValue: Div<\n><obj>
AXRole: AXButton
- AXRole: AXHeading AXValue: Heading<\n><obj>button<\n><obj>
+ AXRole: AXHeading AXValue: Heading<\n><obj><\n><obj>
AXRole: AXButton
AXRole: AXImage
AXRole: AXDiv AXValue: End of test
Modified: trunk/LayoutTests/platform/gtk/accessibility/table-with-rules-expected.txt (158427 => 158428)
--- trunk/LayoutTests/platform/gtk/accessibility/table-with-rules-expected.txt 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/LayoutTests/platform/gtk/accessibility/table-with-rules-expected.txt 2013-11-01 10:59:33 UTC (rev 158428)
@@ -84,7 +84,7 @@
AXSize: { 769.000000, 34.000000 }
AXTitle:
AXDescription:
-AXValue: ------------------------------------
+AXValue: <\n>------------------------------------<\n>
AXFocusable: 0
AXFocused: 0
AXSelectable: 0
Modified: trunk/Source/WebCore/ChangeLog (158427 => 158428)
--- trunk/Source/WebCore/ChangeLog 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/Source/WebCore/ChangeLog 2013-11-01 10:59:33 UTC (rev 158428)
@@ -1,3 +1,54 @@
+2013-11-01 Mario Sanchez Prada <[email protected]>
+
+ [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.
+
+ 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.
+
2013-11-01 Alexey Proskuryakov <[email protected]>
Add a Mac WebCrypto implementation of HMAC importKey/sign/verify
Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (158427 => 158428)
--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2013-11-01 10:59:33 UTC (rev 158428)
@@ -652,13 +652,37 @@
if (m_renderer->isText() || mode.childrenInclusion == AccessibilityTextUnderElementMode::TextUnderElementModeIncludeAllChildren) {
// If possible, use a text iterator to get the text, so that whitespace
// is handled consistently.
- if (Node* node = this->node()) {
- if (Frame* frame = node->document().frame()) {
+ Document* nodeDocument = 0;
+ RefPtr<Range> textRange;
+ if (Node* node = m_renderer->node()) {
+ nodeDocument = &node->document();
+ textRange = rangeOfContents(*node);
+ } else {
+ // For anonymous blocks, we work around not having a direct node to create a range from
+ // defining one based in the two external positions defining the boundaries of the subtree.
+ RenderObject* firstChildRenderer = m_renderer->firstChildSlow();
+ RenderObject* lastChildRenderer = m_renderer->lastChildSlow();
+ if (firstChildRenderer && lastChildRenderer) {
+ ASSERT(firstChildRenderer->node());
+ ASSERT(lastChildRenderer->node());
+
+ // We define the start and end positions for the range as the ones right before and after
+ // the first and the last nodes in the DOM tree that is wrapped inside the anonymous block.
+ Node* firstNodeInBlock = firstChildRenderer->node();
+ Position startPosition = positionInParentBeforeNode(firstNodeInBlock);
+ Position endPosition = positionInParentAfterNode(lastChildRenderer->node());
+
+ nodeDocument = &firstNodeInBlock->document();
+ textRange = Range::create(*nodeDocument, startPosition, endPosition);
+ }
+ }
+
+ if (nodeDocument && textRange) {
+ if (Frame* frame = nodeDocument->frame()) {
// catch stale WebCoreAXObject (see <rdar://problem/3960196>)
- if (frame->document() != &node->document())
+ if (frame->document() != nodeDocument)
return String();
-
- return plainText(rangeOfContents(*node).get(), textIteratorBehaviorForTextRange());
+ return plainText(textRange.get(), textIteratorBehaviorForTextRange());
}
}
Modified: trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp (158427 => 158428)
--- trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp 2013-11-01 10:59:33 UTC (rev 158428)
@@ -69,103 +69,6 @@
return webkitAccessibleGetAccessibilityObject(WEBKIT_ACCESSIBLE(text));
}
-static gchar* textForRenderer(RenderObject* renderer)
-{
- GString* resultText = g_string_new(0);
-
- if (!renderer)
- return g_string_free(resultText, FALSE);
-
- // For RenderBlocks, piece together the text from the RenderText objects they contain.
- for (RenderObject* object = renderer->firstChildSlow(); object; object = object->nextSibling()) {
- if (object->isBR()) {
- g_string_append(resultText, "\n");
- continue;
- }
-
- RenderText* renderText;
- if (object->isText())
- renderText = toRenderText(object);
- else {
- // List item's markers will be treated in an special way
- // later on this function, so ignore them here.
- if (object->isReplaced() && !object->isListMarker())
- g_string_append_unichar(resultText, objectReplacementCharacter);
-
- // We need to check children, if any, to consider when
- // current object is not a text object but some of its
- // children are, in order not to miss those portions of
- // text by not properly handling those situations
- if (object->firstChildSlow()) {
- GOwnPtr<char> objectText(textForRenderer(object));
- g_string_append(resultText, objectText.get());
- }
- continue;
- }
-
- InlineTextBox* box = renderText ? renderText->firstTextBox() : 0;
- while (box) {
- // WebCore introduces line breaks in the text that do not reflect
- // the layout you see on the screen, replace them with spaces.
- String text = String(renderText->characters(), renderText->textLength()).replace("\n", " ");
- g_string_append(resultText, text.substring(box->start(), box->end() - box->start() + 1).utf8().data());
-
- // Newline chars in the source result in separate text boxes, so check
- // before adding a newline in the layout. See bug 25415 comment #78.
- // If the next sibling is a BR, we'll add the newline when we examine that child.
- if (!box->nextOnLineExists() && !(object->nextSibling() && object->nextSibling()->isBR())) {
- // If there was a '\n' in the last position of the
- // current text box, it would have been converted to a
- // space in String::replace(), so remove it first.
- if (renderText->characters()[box->end()] == '\n')
- g_string_erase(resultText, resultText->len - 1, -1);
-
- g_string_append(resultText, "\n");
- }
- box = box->nextTextBox();
- }
- }
-
- // Insert the text of the marker for list item in the right place, if present
- if (renderer->isListItem()) {
- String markerText = toRenderListItem(renderer)->markerTextWithSuffix();
- if (renderer->style().direction() == LTR)
- g_string_prepend(resultText, markerText.utf8().data());
- else
- g_string_append(resultText, markerText.utf8().data());
- }
-
- return g_string_free(resultText, FALSE);
-}
-
-static gchar* textForObject(const AccessibilityObject* coreObject)
-{
- GString* str = g_string_new(0);
-
- // For text controls, we can get the text line by line.
- if (coreObject->isTextControl()) {
- unsigned textLength = coreObject->textLength();
- int lineNumber = 0;
- PlainTextRange range = coreObject->doAXRangeForLine(lineNumber);
- while (range.length) {
- String lineText = coreObject->doAXStringForRange(range);
- g_string_append(str, lineText.utf8().data());
-
- // When a line of text wraps in a text area, the final space
- // after each non-final line must be replaced with a line break.
- if (range.start + range.length < textLength)
- g_string_overwrite_len(str, str->len - 1, "\n", 1);
-
- range = coreObject->doAXRangeForLine(++lineNumber);
- }
- } else if (coreObject->isAccessibilityRenderObject()) {
- GOwnPtr<gchar> rendererText(textForRenderer(coreObject->renderer()));
- g_string_append(str, rendererText.get());
- }
-
- return g_string_free(str, FALSE);
-}
-
static int baselinePositionForRenderObject(RenderObject* renderObject)
{
// FIXME: This implementation of baselinePosition originates from RenderObject.cpp and was
@@ -588,14 +491,6 @@
ret = coreObject->textUnderElement(AccessibilityTextUnderElementMode(AccessibilityTextUnderElementMode::TextUnderElementModeIncludeAllChildren));
}
- if (!ret.length()) {
- // This can happen at least with anonymous RenderBlocks (e.g. body text amongst paragraphs)
- // In such instances, there may also be embedded objects. The object replacement character
- // is something ATs want included and we have to account for the fact that it is multibyte.
- GOwnPtr<char> objectText(textForObject(coreObject));
- ret = String::fromUTF8(objectText.get());
- }
-
// Prefix a item number/bullet if needed
int actualEndOffset = endOffset == -1 ? ret.length() : endOffset;
if (coreObject->roleValue() == ListItemRole) {
Modified: trunk/Source/WebCore/editing/TextIterator.cpp (158427 => 158428)
--- trunk/Source/WebCore/editing/TextIterator.cpp 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/Source/WebCore/editing/TextIterator.cpp 2013-11-01 10:59:33 UTC (rev 158428)
@@ -722,6 +722,9 @@
if (m_emitsObjectReplacementCharacters && renderer && renderer->isReplaced()) {
emitCharacter(objectReplacementCharacter, m_node->parentNode(), m_node, 0, 1);
+ // Don't process subtrees for embedded objects. If the text there is required,
+ // it must be explicitly asked by specifying a range falling inside its boundaries.
+ m_handledChildren = true;
return true;
}
Modified: trunk/Source/WebCore/editing/TextIterator.h (158427 => 158428)
--- trunk/Source/WebCore/editing/TextIterator.h 2013-11-01 07:15:45 UTC (rev 158427)
+++ trunk/Source/WebCore/editing/TextIterator.h 2013-11-01 10:59:33 UTC (rev 158428)
@@ -187,7 +187,7 @@
bool m_handledFirstLetter;
// Used when the visibility of the style should not affect text gathering.
bool m_ignoresStyleVisibility;
- // Used when emitting the special 0xFFFC character is required.
+ // Used when emitting the special 0xFFFC character is required. Children for replaced objects will be ignored.
bool m_emitsObjectReplacementCharacters;
// Used when the iteration should stop if form controls are reached.
bool m_stopsOnFormControls;
