Bug 157644 - AX: Wrong CharacterOffset from an upstream VisiblePosition
Summary: AX: Wrong CharacterOffset from an upstream VisiblePosition
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-05-12 15:59 PDT by Nan Wang
Modified: 2016-05-13 11:56 PDT (History)
11 users (show)

See Also:


Attachments
Initial patch (3.34 KB, patch)
2016-05-12 16:15 PDT, Nan Wang
no flags Details | Formatted Diff | Diff
patch (6.95 KB, patch)
2016-05-13 11:02 PDT, Nan Wang
cfleizach: review+
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-05-12 15:59:28 PDT
When we are getting a text marker at the line wrap and it has an upstream VisiblePosition, we should adjust the CharacterOffset as well.

<rdar://problem/25993225>
Comment 1 Nan Wang 2016-05-12 16:15:39 PDT
Created attachment 278776 [details]
Initial patch

Spent quite a long time but was not able to create a layout test.
1. In Mail the end text marker of left line range has upstream affinity but I tried left/right and regular line text marker range call in layout test it gave downstream affinity all the time.
2. Although I had the fix to adjust CharacterOffset to match Range's end offset, TextIterator will always include an extra space at the line wrap. So there's no easy way to test this fix in a layout test by using lengthForTextMarkerRange of stringForTextMarkerRange, they all use TextIterator under the hood.
Comment 2 chris fleizach 2016-05-12 17:47:38 PDT
Comment on attachment 278776 [details]
Initial patch

When I do a grep for upstream in LayoutTests, I see 

platform/ios-simulator-wk1/editing/pasteboard/4242293-expected.txt:caret: position 3 of child 0 {#text} of child 1 {SPAN} of child 6 {DIV} of body (upstream affinity)
Comment 3 chris fleizach 2016-05-13 08:49:23 PDT
(In reply to comment #2)
> Comment on attachment 278776 [details]
> Initial patch
> 
> When I do a grep for upstream in LayoutTests, I see 
> 
> platform/ios-simulator-wk1/editing/pasteboard/4242293-expected.txt:caret:
> position 3 of child 0 {#text} of child 1 {SPAN} of child 6 {DIV} of body
> (upstream affinity)

Do you think that test might help you figure out how to create an upstream position?
Comment 4 Darin Adler 2016-05-13 08:53:10 PDT
Comment on attachment 278776 [details]
Initial patch

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

> Source/WebCore/ChangeLog:11
> +        Not able to create an upstream VisiblePosition in a layout test.

Why is that? Can we please add something to internals so we can test this? This super-subtle behavior seems critical to test; people will break it without realizing it.
Comment 5 Nan Wang 2016-05-13 10:12:24 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 278776 [details]
> > Initial patch
> > 
> > When I do a grep for upstream in LayoutTests, I see 
> > 
> > platform/ios-simulator-wk1/editing/pasteboard/4242293-expected.txt:caret:
> > position 3 of child 0 {#text} of child 1 {SPAN} of child 6 {DIV} of body
> > (upstream affinity)
> 
> Do you think that test might help you figure out how to create an upstream
> position?

Yes I can create an upstream position by adding contenteditable="true"
Comment 6 Nan Wang 2016-05-13 11:02:49 PDT
Created attachment 278851 [details]
patch

Added layout test. Thanks to Chris!
Comment 7 Nan Wang 2016-05-13 11:56:46 PDT
Committed r200872: <http://trac.webkit.org/changeset/200872>