Bug 153292 - AX: [IOS] Implement next/previous text marker functions using TextIterator
Summary: AX: [IOS] Implement next/previous text marker functions using TextIterator
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-01-20 16:04 PST by Nan Wang
Modified: 2016-01-21 00:35 PST (History)
3 users (show)

See Also:

Attachments
patch (66.85 KB, patch)
2016-01-20 17:48 PST, Nan Wang 		no flags Details | Formatted Diff | Diff
patch (66.75 KB, patch)
2016-01-20 18:24 PST, Nan Wang 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nan Wang 2016-01-20 16:04:49 PST 
Try to use TextIterator class to refactor the next/previous text marker functions on iOS. And also make text marker tests working on iOS.
Comment 1 Radar WebKit Bug Importer 2016-01-20 16:05:47 PST 
<rdar://problem/24268243>
Comment 2 Nan Wang 2016-01-20 17:48:06 PST 
Created attachment 269409 [details]
patch
Comment 3 chris fleizach 2016-01-20 18:00:15 PST 
Comment on attachment 269409 [details]
patch

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

> Source/WebCore/ChangeLog:11
> +        Also, fixed an issue in AXObjectCache that creating a range with a replaced node

AXObjectCache that creating - >AXObjectCache where creating

did you add a test case for this issue you fixed?

> Source/WebCore/accessibility/AXObjectCache.cpp:1551
> +    

remove unused line

> Source/WebCore/accessibility/AXObjectCache.cpp:1577
> +    bool startNodeIsReplacedOrBR = AccessibilityObject::replacedNodeNeedsCharacter(startNode) || startNode->hasTagName(brTag);

can you put this code block in a helper method

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:2506
> +    if (!start || !end)

can you make a helper method in WebAccessibilityTextMarker that takes a Range and returns an array of the WebAccessibilityTextMarker

> Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:485
> +    return 0;

return nullptr;

> Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:490
> +    return 0;

return nullptr;

> Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:495
> +    return 0;

return nullptr;

> Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:535
> +    return 0;

return nullptr;
Comment 4 Nan Wang 2016-01-20 18:04:40 PST 
Comment on attachment 269409 [details]
patch

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

Will address these comments. Thanks.

>> Source/WebCore/ChangeLog:11
>> +        Also, fixed an issue in AXObjectCache that creating a range with a replaced node
> 
> AXObjectCache that creating - >AXObjectCache where creating
> 
> did you add a test case for this issue you fixed?

Yes, it's covered in the test when calling textMarkerRangeForMarkers.
Comment 5 Nan Wang 2016-01-20 18:24:26 PST 
Created attachment 269411 [details]
patch

review comments
Comment 6 WebKit Commit Bot 2016-01-21 00:35:22 PST 
Comment on attachment 269411 [details]
patch

Clearing flags on attachment: 269411

Committed r195405: <http://trac.webkit.org/changeset/195405>
Comment 7 WebKit Commit Bot 2016-01-21 00:35:26 PST 
All reviewed patches have been landed.  Closing bug.