Bug 92812 - Smart link can erroneously move caret after an URL when typing immediately before it
Summary: Smart link can erroneously move caret after an URL when typing immediately be...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL: http://webkit.org/new-bug
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-07-31 16:39 PDT by Mark Rowe (bdash)
Modified: 2013-02-01 12:30 PST (History)
7 users (show)

See Also:


Attachments
Demo (disable 'Check Spelling While Typing' and enable 'Smart Links') (390 bytes, text/html)
2013-02-01 00:48 PST, Ryosuke Niwa
no flags Details
Same reduction but reproduces the bug in DRT (719 bytes, text/html)
2013-02-01 01:01 PST, Ryosuke Niwa
no flags Details
Patch (4.99 KB, patch)
2013-02-01 02:12 PST, Ryosuke Niwa
enrica: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Rowe (bdash) 2012-07-31 16:39:47 PDT
* 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>
Comment 1 Ryosuke Niwa 2012-10-19 14:07:17 PDT
I can't reproduce this issue anymore.
Comment 2 Mark Rowe (bdash) 2012-10-19 14:14:53 PDT
I just noticed that this only reproduces if you have the Smart Links option enabled in the Substitutions part of the context menu.
Comment 3 Yi Shen 2012-10-26 16:58:44 PDT
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.
Comment 4 Mark Rowe (bdash) 2013-01-29 13:27:14 PST
If you can’t reproduce this, try quitting Safari, doing “defaults write com.apple.Safari WebAutomaticLinkDetectionEnabled -bool YES”, and then attempt to reproduce again.
Comment 5 Mark Rowe (bdash) 2013-01-29 14:15:31 PST
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.

Comment 6 Ryosuke Niwa 2013-02-01 00:41:47 PST
(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".
Comment 7 Ryosuke Niwa 2013-02-01 00:48:02 PST
Created attachment 185970 [details]
Demo (disable 'Check Spelling While Typing' and enable 'Smart Links')
Comment 8 Ryosuke Niwa 2013-02-01 00:50:01 PST
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.
Comment 9 Mark Rowe (bdash) 2013-02-01 00:57:39 PST
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.
Comment 10 Ryosuke Niwa 2013-02-01 01:01:15 PST
Created attachment 185971 [details]
Same reduction but reproduces the bug in DRT
Comment 11 Ryosuke Niwa 2013-02-01 01:49:05 PST
I think I have a fix for this. It's a one-liner :)
Comment 12 Ryosuke Niwa 2013-02-01 02:12:10 PST
Created attachment 185986 [details]
Patch
Comment 13 Ryosuke Niwa 2013-02-01 02:13:01 PST
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.
Comment 14 Build Bot 2013-02-01 06:07:21 PST
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
Comment 15 Ryosuke Niwa 2013-02-01 10:42:18 PST
(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.
Comment 16 Enrica Casucci 2013-02-01 11:34:07 PST
Comment on attachment 185986 [details]
Patch

Looks good to me. Thanks for fixing this!
Comment 17 Ryosuke Niwa 2013-02-01 11:46:05 PST
Committed r141618: <http://trac.webkit.org/changeset/141618>
Comment 18 Jessie Berlin 2013-02-01 12:23:52 PST
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.
Comment 19 Jessie Berlin 2013-02-01 12:30:33 PST
(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