Smart link can erroneously move caret after an URL when typing immediately before it

[Mark Rowe (bdash)]

* SUMMARY
When editing text in a text area that contains a URL, the caret will often jump forward unexpectedly.

* STEPS TO REPRODUCE
1. Load <http://webkit.org/new-bug>
2. Type the text http://webkit.org/ in to the Description field.
3. Press Ctrl-A to move the caret to the beginning of the line, before the "h".
4. Type Before and press space.

* RESULTS
The caret jumps to the end of the URL in the text area.

* NOTES
This broke in <http://trac.webkit.org/changeset/117590>.

<rdar://problem/11994169>

[Ryosuke Niwa]

I can't reproduce this issue anymore.

[Mark Rowe (bdash)]

I just noticed that this only reproduces if you have the Smart Links option enabled in the Substitutions part of the context menu.

[Yi Shen]

Mark, could you double check this issue. I couldn't reproduce it with the latest mac build.
(In reply to comment #2)
> I just noticed that this only reproduces if you have the Smart Links option enabled in the Substitutions part of the context menu.

[Mark Rowe (bdash)]

If you can’t reproduce this, try quitting Safari, doing “defaults write com.apple.Safari WebAutomaticLinkDetectionEnabled -bool YES”, and then attempt to reproduce again.

[Mark Rowe (bdash)]

There's one additional piece that's missing: continuous spell checking must be disabled in order to reproduce.

The following steps are what is necessary in order to reproduce:
1. Load <http://webkit.org/new-bug>
2. Right-click in the Description field, choose Spelling and Grammar and disable Check Spelling While Typing.
3. Right-click in the Description field, choose Substitutions and enable Smart Links.
4. Type the text http://webkit.org/ in to the Description field.
5. Press Ctrl-A to move the caret to the beginning of the line, before the "h".
6. Type Before and press space.


[Ryosuke Niwa]

(In reply to comment #5)
> There's one additional piece that's missing: continuous spell checking must be disabled in order to reproduce.
> 
> The following steps are what is necessary in order to reproduce:
> 1. Load <http://webkit.org/new-bug>
> 2. Right-click in the Description field, choose Spelling and Grammar and disable Check Spelling While Typing.
> 3. Right-click in the Description field, choose Substitutions and enable Smart Links.
> 4. Type the text http://webkit.org/ in to the Description field.
> 5. Press Ctrl-A to move the caret to the beginning of the line, before the "h".
> 6. Type Before and press space.

It seems like in step 6, you need to misspell "before".

[Ryosuke Niwa]

Created attachment 185970 [details]
Demo (disable 'Check Spelling While Typing' and enable 'Smart Links')

[Ryosuke Niwa]

By the way, I'm not sure if it's really accurate to blame http://trac.webkit.org/changeset/117590 here since that patch merely re-enabled smart links. Prior to that patch, smart link was completely broken. It's possible that we had this bug before r117590.

[Mark Rowe (bdash)]

This also reproduces for me in a slightly different form in this "Additional Comments" text field by typing "http://webkit.org foo bar baz". It comes out as "http://webkit.org baz bar foo", as the cursor is moving unexpectedly as I type.

[Ryosuke Niwa]

Created attachment 185971 [details]
Same reduction but reproduces the bug in DRT

[Ryosuke Niwa]

I think I have a fix for this. It's a one-liner :)

[Ryosuke Niwa]

Created attachment 185986 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 185986 [details]
Patch

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

> Source/WebCore/editing/Editor.cpp:2131
> +            if (result->type == TextCheckingTypeLink
> +                && (selectionOffset > resultEnd + 1 || selectionOffset <= resultLocation))

On my second thought, I should probably fit this in one line.

[Build Bot]

Comment on attachment 185986 [details]
Patch

Attachment 185986 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16298627

New failing tests:
editing/inserting/smart-link-when-caret-is-moved-before-URL.html

[Ryosuke Niwa]

(In reply to comment #14)
> (From update of attachment 185986 [details])
> Attachment 185986 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://queues.webkit.org/results/16298627
> 
> New failing tests:
> editing/inserting/smart-link-when-caret-is-moved-before-URL.html

This test probably needs to be disabled on WK2 due to WTR's missing method.

[Enrica Casucci]

Comment on attachment 185986 [details]
Patch

Looks good to me. Thanks for fixing this!

[Ryosuke Niwa]

Committed r141618: <http://trac.webkit.org/changeset/141618>

[Jessie Berlin]

Note: editing/inserting/smart-link-when-caret-is-moved-before-URL.html uses setAutomaticLinkDetectionEnabled, which is not implemented for WebKitTestRunner (https://bugs.webkit.org/show_bug.cgi?id=87162). I am going to skip that test in the wk2 TestExpectations file.

[Jessie Berlin]

(In reply to comment #18)
> Note: editing/inserting/smart-link-when-caret-is-moved-before-URL.html uses setAutomaticLinkDetectionEnabled, which is not implemented for WebKitTestRunner (https://bugs.webkit.org/show_bug.cgi?id=87162). I am going to skip that test in the wk2 TestExpectations file.

Skipped for wk2 in http://trac.webkit.org/changeset/141627