Bug 191306 – Automatic link replacement via "Smart links" should emit "insertLink" input events

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

[Wenson Hsieh]

To reproduce:

1. https://whsieh.github.io/examples/input-events.
2. Right click, and make sure that Substitutions → Smart links is checked in the context menu.
3. Type "https://www.apple.com" and a space.

Expected: input type of "insertLink"
Observed: input type of null

[Radar WebKit Bug Importer]

<rdar://problem/58491514>

[Nikos Mouchtaris]

Created attachment 388344 [details]
Patch

[Tim Horton]

Comment on attachment 388344 [details]
Patch

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

> Source/WebCore/ChangeLog:5
> +

You should stick the radar link here (see other commits for examples).

> LayoutTests/fast/events/emit-insertLink.html:9
> +        description("To manually test this, change the foreground color and check the resulting debug messages.");

Is this right? It doesn't sound right.

[Wenson Hsieh]

Comment on attachment 388344 [details]
Patch

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

> LayoutTests/fast/events/emit-insertLink-expected.txt:18
> +event.type = beforeinput
> +event.inputType = insertText
> +event.data = www.apple.com 
> +event.type = input
> +event.inputType = insertText
> +event.data = www.apple.com 
> +event.type = beforeinput
> +event.inputType = insertParagraph
> +event.data = null
> +event.type = input
> +event.inputType = insertParagraph
> +event.data = null
> +PASS successfullyParsed is true

Hm...I would've expected to see a line that says `event.inputType = insertLink` here too, instead of just insertText and insertParagraph.

[Nikos Mouchtaris]

Created attachment 388351 [details]
Patch

[Wenson Hsieh]

Comment on attachment 388344 [details]
Patch

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

>> LayoutTests/fast/events/emit-insertLink-expected.txt:18
>> +PASS successfullyParsed is true
> 
> Hm...I would've expected to see a line that says `event.inputType = insertLink` here too, instead of just insertText and insertParagraph.

Ah, you might need this:

if (window.internals)
    internals.setAutomaticLinkDetectionEnabled(true);

[Wenson Hsieh]

(In reply to Wenson Hsieh from comment #6)
> Comment on attachment 388344 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388344&action=review
> 
> >> LayoutTests/fast/events/emit-insertLink-expected.txt:18
> >> +PASS successfullyParsed is true
> > 
> > Hm...I would've expected to see a line that says `event.inputType = insertLink` here too, instead of just insertText and insertParagraph.
> 
> Ah, you might need this:
> 
> if (window.internals)
>     internals.setAutomaticLinkDetectionEnabled(true);

...regrettably, this doesn't work in WebKit2; this is a known issue, currently being tracked by <webkit.org/b/162081>.

We could probably make the test work on WebKit2 as a followup in <webkit.org/b/162081>, and only enable in on WebKit1 for the time being.

[Nikos Mouchtaris]

Created attachment 388364 [details]
Patch

[Wenson Hsieh]

Comment on attachment 388364 [details]
Patch

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

> LayoutTests/fast/events/emit-insertLink.html:11
> +        if (window.internals)
> +            internals.settings.setInputEventsEnabled(true);
> +            internals.setAutomaticLinkDetectionEnabled(true);

Missing { } around the multiline if statement.

[Nikos Mouchtaris]

Created attachment 388368 [details]
Patch

[Wenson Hsieh]

Comment on attachment 388368 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Test: fast/events/emit-insertLink.html

Let's update this to fast/events/input-event-insert-link.html as well.

> LayoutTests/ChangeLog:11
> +        * fast/events/emit-insertLink-expected.txt: Added.
> +        * fast/events/emit-insertLink.html: Added.

Let's update these to fast/events/input-event-insert-link.html as well.

> LayoutTests/fast/events/input-event-insert-link.html:14
> +

I think it helps to have a quick description here; perhaps something like, `description("This test verifies that input events of inputType 'insertLink' are dispatched when automatically replacing text with a link.");`.

[Nikos Mouchtaris]

Created attachment 388374 [details]
Patch

[Nikos Mouchtaris]

Created attachment 388465 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 388465 [details]
Patch

Clearing flags on attachment: 388465

Committed r254945: <https://trac.webkit.org/changeset/254945>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.