[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 #9 from chris fleizach <cfleizach 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

> Source/WebCore/accessibility/AXObjectCache.cpp:1457
> +    for (; !iterator.atEnd(); iterator.advance()) {

seems like int count = 1 and count++ can be part of the for loop

> Source/WebCore/accessibility/AXObjectCache.cpp:1466
> +            if (AccessibilityObject::replacedNodeNeedsCharacter(node.traverseToChildAt(subOffset))) {

you can probably cache node.traverseToChildAt(subOffset) in a local variable so you don't have to call it twice

> Source/WebCore/accessibility/AXObjectCache.cpp:1479
> +        if (!currentLength)

this check could be put up as the else for the if block at 1466 right?

> Source/WebCore/accessibility/AXObjectCache.cpp:1562
> +    // endOffset can be out of bound sometimes if the node is a replaced node or has brTag.

out of "bounds"

> Source/WebCore/accessibility/AXObjectCache.cpp:1565
> +        if (endOffset > TextIterator::rangeLength(nodeRange.get()))

you can store TextIterator::rangeLength(nodeRange.get()) so you don't have to call it twice

> Source/WebCore/accessibility/AXObjectCache.h:55
> +    int characterOffset;

what is startIndex and what is ignored used for?

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:805
> +        return nil;

nullptr since it's a C++ object

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:847
> +    if (isTextMarkerIgnored(textMarker))

doesn't this need to be in a while loop

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:856
> +    if (isTextMarkerIgnored(textMarker))

doesn't this need to be in a while loop

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:877
> +    if (!textMarkerData.axID && !textMarkerData.ignored)

can we use axID = 0 to mean ignored?

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:886
> +        return nil;

nullptr since its C++

> LayoutTests/accessibility/mac/previous-next-text-marker.html:117
> +        shouldBeTrue("!psw.accessibilityElementForTextMarker(start)");

shouldBeFalse

-- 
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/7e7947fe/attachment.html>