[Webkit-unassigned] [Bug 119173] New: AccessibilityObject::stringForVisiblePositionRange() incorrectly repeats list marker text

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=119173

           Summary: AccessibilityObject::stringForVisiblePositionRange()
                    incorrectly repeats list marker text
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: Unspecified
        OS/Version: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Accessibility
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: jdiggs at igalia.com
                CC: cfleizach at apple.com,
                    webkit-bug-importer at group.apple.com


Working on a downstream bug, I am seeing the list marker text inserted multiple times:

* At the start of the list item (as expected)
* At the start (and sometimes also at the end) of a wrapped line (not expected)
* Before/after embedded images, spans, etc (not expected)

Looking at AccessibilityObject::stringForVisiblePositionRange() [1], I noticed that for each TextIterator, we're calling AccessibilityObject::listMarkerTextForNodeAndPosition() with visiblePositionRange.start.

AccessibilityObject::listMarkerTextForNodeAndPosition() looks at that VisiblePosition, and only returns an empty String if that VisiblePosition is not the start of the line or there is no RenderListItem container for the node.

Why are we passing in the same VisiblePosition for each TextIterator in the VisiblePositionRange? That seems to be the culprit for the bug in question.

The following non-patch is just something I tried as part of debugging. It solves the problem. But without knowing why the original implementation was done as it was, I have no idea what this change might break. Chris, if you have some time to enlighten me, I'd be most grateful. Thanks in advance!

[1] http://trac.webkit.org/browser/trunk/Source/WebCore/accessibility/AccessibilityObject.cpp#L859
[2] http://trac.webkit.org/browser/trunk/Source/WebCore/accessibility/AccessibilityObject.cpp#L843

------------------------------------------------------------------------

diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
index 9d564d0..162f24e 100644
--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
+++ b/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -857,11 +857,12 @@ String AccessibilityObject::stringForVisiblePositionRange(const VisiblePositionR

     StringBuilder builder;
     RefPtr<Range> range = makeRange(visiblePositionRange.start, visiblePositionRange.end);
+    VisiblePosition position = visiblePositionRange.start;
     for (TextIterator it(range.get()); !it.atEnd(); it.advance()) {
         // non-zero length means textual node, zero length means replaced node (AKA "attachments" in AX)
         if (it.length()) {
             // Add a textual representation for list marker text
-            String listMarkerText = listMarkerTextForNodeAndPosition(it.node(), visiblePositionRange.start);
+            String listMarkerText = listMarkerTextForNodeAndPosition(it.node(), position);
             if (!listMarkerText.isEmpty())
                 builder.append(listMarkerText);

@@ -876,6 +877,7 @@ String AccessibilityObject::stringForVisiblePositionRange(const VisiblePositionR
             if (replacedNodeNeedsCharacter(node->childNode(offset)))
                 builder.append(objectReplacementCharacter);
         }
+        position = position.next();
     }

     return builder.toString();

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.