[Webkit-unassigned] [Bug 152728] AX: [Mac] Implement next/previous text marker functions using TextIterator

[bugzilla-daemon at webkit.org]

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

--- Comment #10 from Nan Wang <n_wang at apple.com> ---
Comment on attachment 268922
  --> https://bugs.webkit.org/attachment.cgi?id=268922
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=268922&action=review

I'll address other comments.

>> Source/WebCore/accessibility/AXObjectCache.h:55
>> +    int characterOffset;
> 
> what is startIndex and what is ignored used for?

In the range of a text node's content, the start offset is not always 0. So I used (startIndex + character count) to set the correct start offset when creating a range from text marker range.
Ignored is used here for password field, so we can skip that when calling next/previous text marker call.

>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:847
>> +    if (isTextMarkerIgnored(textMarker))
> 
> doesn't this need to be in a while loop

I'm calling nextTextMarkerForNode:, not textMarkerForNode:. So it's recursive.

>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:856
>> +    if (isTextMarkerIgnored(textMarker))
> 
> doesn't this need to be in a while loop

ditto.

>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:877
>> +    if (!textMarkerData.axID && !textMarkerData.ignored)
> 
> can we use axID = 0 to mean ignored?

We also return an empty TextMarkerData when node is nullptr. In that case we want to return nil for the text marker. For example we get to the end of document that we can't ignore it and keep going forward. But here for password filed, we want to treat it differently, so that we know the node exists and there might be nodes after it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160114/af1e4bdd/attachment-0001.html>