Bug 157190 - AX: CharacterOffset not working correctly with composed characters and collapsed white spaces
Summary: AX: CharacterOffset not working correctly with composed characters and collap...
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-04-29 11:01 PDT by Nan Wang
Modified: 2016-04-29 13:04 PDT (History)
11 users (show)

See Also:


Attachments
Initial patch (16.64 KB, patch)
2016-04-29 11:13 PDT, Nan Wang
no flags Details | Formatted Diff | Diff
patch (19.82 KB, patch)
2016-04-29 11:59 PDT, 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-04-29 11:01:08 PDT
When navigating emoji by character, CharacterOffset is using wrong offset to create Range and result in the bad behavior.
Same goes for collapsed white spaces, we should use the real space sequence length to create Range.

<rdar://problem/25919648>
Comment 1 Nan Wang 2016-04-29 11:13:35 PDT
Created attachment 277720 [details]
Initial patch
Comment 2 chris fleizach 2016-04-29 11:40:13 PDT
Comment on attachment 277720 [details]
Initial patch

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

> Source/WebCore/ChangeLog:9
> +        using the helper function in Position to get the real character count for the composed charecter sequence.

charecter -> character

> Source/WebCore/ChangeLog:11
> +        use the actuall space length to create the CharacterOffset in order to generate valid Range object from it.

actuall

> Source/WebCore/accessibility/AXObjectCache.cpp:1557
> +                // When there's collapsed whitespace, text iterator will only count those spaces as one single space.

the text iterator

> Source/WebCore/accessibility/AXObjectCache.cpp:1564
> +                    offsetSoFar += appendLength;

offsetSoFar -> cumulativeOffset

> Source/WebCore/accessibility/AXObjectCache.cpp:1981
> +        Position previousVpDeepPos = previousVisiblePos.deepEquivalent();

Vp looks weird to me

can you call this *visiblePosition*

> Source/WebCore/accessibility/AXObjectCache.cpp:2058
> +    // We don't always move one 'character' a time since there might be composed characters.

at a time
Comment 3 Nan Wang 2016-04-29 11:59:44 PDT
Created attachment 277725 [details]
patch

Applied review comments
Comment 4 WebKit Commit Bot 2016-04-29 13:04:36 PDT
Comment on attachment 277725 [details]
patch

Clearing flags on attachment: 277725

Committed r200258: <http://trac.webkit.org/changeset/200258>
Comment 5 WebKit Commit Bot 2016-04-29 13:04:41 PDT
All reviewed patches have been landed.  Closing bug.