Bug 130906 - AX: Need ability to get line range for text marker.
Summary: AX: Need ability to get line range for text marker.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.9
: P2 Normal
Assignee: Samuel White
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-03-28 10:59 PDT by Samuel White
Modified: 2014-03-31 11:05 PDT (History)
11 users (show)

See Also:


Attachments
Patch for EWS. (15.61 KB, patch)
2014-03-28 12:28 PDT, Samuel White
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (497.42 KB, application/zip)
2014-03-28 13:57 PDT, Build Bot
no flags Details
Patch. (20.75 KB, patch)
2014-03-28 14:22 PDT, Samuel White
cfleizach: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.