Bug 28200 – ListMarker should be included as part of the text value to parse

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

If a list includes list markers, the list marker values need to be included as part of the text string.

As an example, if bullet character is at the beginning of every list item, then each one of those bullet character needs to be part of the AXValue of the AXWebArea.  And asking any of the text parsing SPIs, such as AXStringForRange, should include the bullet character.

[chris fleizach]

Created attachment 34611 [details]
patch

[Darin Adler]

Comment on attachment 34611 [details]
patch

> +        Node* stringNode = node;
> +        while (stringNode) {

We'd normally write this as a for loop.

           for (Node* stringNode = node; stringNode; stringNode = stringNode->parent())

> +            RenderObject* renderObject = stringNode->renderer();
> +            if (renderObject->isListItem()) {

You need to check the renderObject for 0 here. There's not an ironclad guarantee it will be non-zero. If you were walking up the render tree then there would be, but a DOM parent of a renderer might not be rendered. I am not even sure this will do the right thing in unusual circumstances, like when a list is in a floating element. DOM parents might be far away from the element in question, so walking up the DOM tree like this is not necessarily a good idea.

I think the code would read better if you factored the code to find the RenderListItem* apart from the code to extract the marker text, maybe even in a separate function. It could help make the flow clearer to not have the marker text code inside a loop.

> +    String listMarkerTextForRange(const VisiblePosition& , Node*) const;

Extra space here before the comma. Strange that this takes both a VisiblePosition and a Node*. I'm not really sure I understand the nature and relationship of the two arguments.

> +    if ([string isKindOfClass:[NSString class]])
> +        return [string createJSStringRef];
> +    return 0;

We normally prefer to have the early exit be for the failure case.

> +JSStringRef AccessibilityUIElement::stringForRange(unsigned location, unsigned length)

Typically we omit argument names if they are not used in a function. I guess we don't have that warning turned on here, but it's worth following the style for new code.

r=me

[chris fleizach]

http://trac.webkit.org/changeset/47076