Bug 115070 – Moving word boundaries backwards fails when there is a text node starting with an apostrophe

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

Moving word boundaries backwards fails when there is a text node starting with an apostrophe

[Ryosuke Niwa]

Created attachment 199343 [details]
Fixes the bug

[Alexey Proskuryakov]

Comment on attachment 199343 [details]
Fixes the bug

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

r=me

> Source/WebCore/ChangeLog:12
> +        text node is seen by ICU on its own even though there is no word boundary between the two when the entire
> +        "I'll" is given to ICU.

I think that it would be more appropriate to talk about the expected behavior per Unicode spec here, not about ICU, which is an implementation detail (and not even used on Mac in this case, at least not directly).

> Source/WebCore/ChangeLog:15
> +        work around this bug. In the long term, we probably need to give ICU/AppKit the entire context since in

More precisely, Foundation - I don't think that this part of NSString is in AppKit.

> Source/WebCore/ChangeLog:16
> +        langugaes like Hebrew and some of European languages, there could be many accents and combining characters

typo: langugaes

> Source/WebCore/editing/VisibleUnits.cpp:502
> +        if (next > 1) // FIXME: This is a work around for https://webkit.org/b/115070. We need to give more contexts in general case.

I'd say "provide more context", not "give".

> LayoutTests/editing/selection/previous-word-boundary-across-text-nodes.html:39
> +    shouldBeEqualToString("selectWordBackward(container); /* " + container.innerHTML + " */ getSelection().toString();", container.textContent);

Does the commented out code help here?

[Ryosuke Niwa]

Comment on attachment 199343 [details]
Fixes the bug

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

Thanks for the review.

>> Source/WebCore/ChangeLog:12
>> +        "I'll" is given to ICU.
> 
> I think that it would be more appropriate to talk about the expected behavior per Unicode spec here, not about ICU, which is an implementation detail (and not even used on Mac in this case, at least not directly).

What specifically did you have in your mind?

> LayoutTests/editing/selection/previous-word-boundary-across-text-nodes-expected.txt:24
> +I'll

Oops, I forgot to hide these nodes after the test. Will do that.

>> LayoutTests/editing/selection/previous-word-boundary-across-text-nodes.html:39
>> +    shouldBeEqualToString("selectWordBackward(container); /* " + container.innerHTML + " */ getSelection().toString();", container.textContent);
> 
> Does the commented out code help here?

Yeah, it gets printed out in the expected result so that we can see what’s being tested.

[Ryosuke Niwa]

Committed r149058: <http://trac.webkit.org/changeset/149058>