Bug 154312

Summary:

AX: Implement sentence related text marker functions using TextIterator

Product:

WebKit

Reporter:

Nan Wang <n_wang>

Component:

Accessibility

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cfleizach, commit-queue, n_wang, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
patch	none
patch	none

Nan Wang

Reported 2016-02-16 15:14:53 PST

Improve accessibility text navigation. Change the sentence related text marker calls to use TextIterator.

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Nan Wang

Comment 1 2016-02-16 15:15:05 PST

<rdar://problem/24269658>

Nan Wang

Comment 2 2016-02-16 15:37:49 PST

Created attachment 271497 [details] patch

chris fleizach

Comment 3 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

Nan Wang

Comment 4 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.

Nan Wang

Comment 5 2016-02-17 00:34:07 PST

Created attachment 271545 [details] patch

WebKit Commit Bot

Comment 6 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>

WebKit Commit Bot

Comment 7 2016-02-17 10:16:53 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.