Bug 154312 - AX: Implement sentence related text marker functions using TextIterator
Summary: AX: Implement sentence related 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-02-16 15:14 PST by Nan Wang
Modified: 2016-02-17 10:16 PST (History)
4 users (show)

See Also:


Attachments
patch (50.48 KB, patch)
2016-02-16 15:37 PST, Nan Wang
no flags Details | Formatted Diff | Diff
patch (51.04 KB, patch)
2016-02-17 00:34 PST, Nan Wang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nan Wang 2016-02-16 15:14:53 PST
Improve accessibility text navigation. 
Change the sentence related text marker calls to use TextIterator.
Comment 1 Nan Wang 2016-02-16 15:15:05 PST
<rdar://problem/24269658>
Comment 2 Nan Wang 2016-02-16 15:37:49 PST
Created attachment 271497 [details]
patch
Comment 3 chris fleizach 2016-02-16 17:30:07 PST
Comment on attachment 271497 [details]
patch

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

looks ok

> Source/WebCore/accessibility/AXObjectCache.cpp:1741
> +            if (charOffset.offset == offset)

do we have a test case covering this now?

> Source/WebCore/accessibility/AXObjectCache.cpp:2122
> +    // We don't want to go to the previous node.

can you add why

> Source/WebCore/accessibility/AXObjectCache.cpp:2193
> +    if (!next.isNull() && next.node->hasTagName(brTag) && !characterOffset.node->hasTagName(brTag))

can you put this if into a static method. looks like you use it here and below

> Source/WebCore/accessibility/AXObjectCache.cpp:2230
> +    // make sure we move off of a sentence end

full sentence
Comment 4 Nan Wang 2016-02-16 17:33:37 PST
Comment on attachment 271497 [details]
patch

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

>> Source/WebCore/accessibility/AXObjectCache.cpp:1741
>> +            if (charOffset.offset == offset)
> 
> do we have a test case covering this now?

Yes, it's covered in the sentence navigation.

>> Source/WebCore/accessibility/AXObjectCache.cpp:2122
>> +    // We don't want to go to the previous node.
> 
> can you add why

Ok.

>> Source/WebCore/accessibility/AXObjectCache.cpp:2193
>> +    if (!next.isNull() && next.node->hasTagName(brTag) && !characterOffset.node->hasTagName(brTag))
> 
> can you put this if into a static method. looks like you use it here and below

Ok.

>> Source/WebCore/accessibility/AXObjectCache.cpp:2230
>> +    // make sure we move off of a sentence end
> 
> full sentence

Ok.
Comment 5 Nan Wang 2016-02-17 00:34:07 PST
Created attachment 271545 [details]
patch
Comment 6 WebKit Commit Bot 2016-02-17 10:16:49 PST
Comment on attachment 271545 [details]
patch

Clearing flags on attachment: 271545

Committed r196699: <http://trac.webkit.org/changeset/196699>
Comment 7 WebKit Commit Bot 2016-02-17 10:16:53 PST
All reviewed patches have been landed.  Closing bug.