Bug 130906

Summary: AX: Need ability to get line range for text marker.
Product: WebKit Reporter: Samuel White <samuel_white>
Component: AccessibilityAssignee: Samuel White <samuel_white>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, buildbot, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, mario, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.9   
Attachments:
Description Flags
Patch for EWS.
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
none
Patch. cfleizach: review+

Description Samuel White 2014-03-28 10:59:19 PDT
We need the ability to fetch the range of an entire line given any arbitrary text marker on that line. Currently, getting the range of a line given a marker can be pretty expensive and requires a handful of IPC calls. We already have this ability for paragraphs and sentences so this is a natural 'next' step.

I'm thinking:
AXLineTextMarkerRangeForTextMarker
Comment 1 Radar WebKit Bug Importer 2014-03-28 11:00:18 PDT
<rdar://problem/16458741>
Comment 2 Radar WebKit Bug Importer 2014-03-28 11:01:04 PDT
<rdar://problem/16458746>
Comment 3 chris fleizach 2014-03-28 11:14:56 PDT
(In reply to comment #0)
> We need the ability to fetch the range of an entire line given any arbitrary text marker on that line. Currently, getting the range of a line given a marker can be pretty expensive and requires a handful of IPC calls. We already have this ability for paragraphs and sentences so this is a natural 'next' step.
> 
> I'm thinking:
> AXLineTextMarkerRangeForTextMarker

If you're getting the text back, I'd say LineText. If you're getting a TextMarkerRange back, maybe better would be

AXLineMarkerRangeForTextMarker
Comment 4 Samuel White 2014-03-28 12:28:59 PDT
Created attachment 228077 [details]
Patch for EWS.

Just looking for platform failures. Will address naming feedback in next patch.
Comment 5 Samuel White 2014-03-28 12:37:26 PDT
(In reply to comment #3)
> (In reply to comment #0)
> > We need the ability to fetch the range of an entire line given any arbitrary text marker on that line. Currently, getting the range of a line given a marker can be pretty expensive and requires a handful of IPC calls. We already have this ability for paragraphs and sentences so this is a natural 'next' step.
> > 
> > I'm thinking:
> > AXLineTextMarkerRangeForTextMarker
> 
> If you're getting the text back, I'd say LineText. If you're getting a TextMarkerRange back, maybe better would be
> 
> AXLineMarkerRangeForTextMarker

After some additional consideration, I think we should keep the original name to better match existing similar attributes. Specifically:

AXParagraphTextMarkerRangeForTextMarker
AXSentenceTextMarkerRangeForTextMarker

Does this sound ok Chris, or do you think leaving 'text' may cause confusion?
Comment 6 chris fleizach 2014-03-28 12:43:18 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > (In reply to comment #0)
> > > We need the ability to fetch the range of an entire line given any arbitrary text marker on that line. Currently, getting the range of a line given a marker can be pretty expensive and requires a handful of IPC calls. We already have this ability for paragraphs and sentences so this is a natural 'next' step.
> > > 
> > > I'm thinking:
> > > AXLineTextMarkerRangeForTextMarker
> > 
> > If you're getting the text back, I'd say LineText. If you're getting a TextMarkerRange back, maybe better would be
> > 
> > AXLineMarkerRangeForTextMarker
> 
> After some additional consideration, I think we should keep the original name to better match existing similar attributes. Specifically:
> 
> AXParagraphTextMarkerRangeForTextMarker
> AXSentenceTextMarkerRangeForTextMarker
> 
> Does this sound ok Chris, or do you think leaving 'text' may cause confusion?

If those already exist, we might as well stick with that convention
Comment 7 Build Bot 2014-03-28 13:57:36 PDT
Comment on attachment 228077 [details]
Patch for EWS.

Attachment 228077 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5248652536709120

New failing tests:
platform/mac/accessibility/bounds-for-range.html
Comment 8 Build Bot 2014-03-28 13:57:39 PDT
Created attachment 228082 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 9 Samuel White 2014-03-28 14:22:41 PDT
Created attachment 228083 [details]
Patch.

Revised patch with build and test fixes. Also includes change log updates.
Comment 10 chris fleizach 2014-03-28 14:27:49 PDT
Comment on attachment 228083 [details]
Patch.

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

r=me assuming tests all pass

> Tools/DumpRenderTree/AccessibilityUIElement.cpp:1351
> +    return 0;

nullptr
Comment 11 Samuel White 2014-03-31 11:05:22 PDT
Committed manually with requested change:
http://trac.webkit.org/changeset/166513

Closing.