Added autolink typing support

[jacekbogdanski]

What is the purpose of this pull request?

New feature

Does your PR contain necessary tests?

All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.

This PR contains

• Unit tests
• Manual tests

What changes did you make?

I used textmatch plugin to provide autolink typing support. Pattern matching is a little complicated but it's required if we want to utilize the same URL/Email regex for both pastings from clipboard and typing.

Closes #1815

[Comandeer]

I don't look into code as there are several issues with the behaviour:

1. After autolinking, there is no undo step to return to unlinked text.
2. I'm not sure, if all the commit keys should be supported. E.g. pressing left arrow suggest that I made mistake in the text I've just written. There is no need to convert it into link. Right arrow in most cases won't do anything as link will be the last element inside the editor. I also have doubts about up arrow and down arrow, end, home, page down and page up. These keys are used for scrolling content. It's quite unexpected that content is replaced while pressing them. Word replaces links
3. At the same moment Tab is not working as it focuses the next element after the editor, even when tab plugin is loaded.

[Comandeer]

Actually it's no longer in HTML 5.x, now it's only in HTML LS.

[jacekbogdanski]

I refactored code to expose autolink_commitKeystrokes configuration property and removed redundant keystrokes. With the current implementation, a user can decide which keystrokes use to accept link completion. Defaults are enter and space.

I also changed undo behavior - now it requires 3 undo steps to remove a link:

• undo commit keystroke if it creates undo step like space or enter
• undo link
• undo link text

[Comandeer]

1. The third undo step seems totally unnecessary and confusing, especially when user typed something after the link:

   1. Type https://cksource.com.
   2. Press Space.
   3. Type something.
   4. Press Undo twice.

   Instead of unlinking we just deleted space after the link.

2. Enter seems to stop working in built version of editor. It's probably connected with enterkey plugin.

3. Can't the part of new logic (especially connected with textmatch plugin) be used inside paste handler? In fact I'm quite sure that binding current keydown handler (without, of course, check for commit keys) to afterPaste event would do the trick and would greatly reduce complexity of the whole plugin.

4. There is some strange bug that removes part of the content between two links if they are divided by one word.

   1. Type https://cksource.com.
   2. Press Space.
   3. Type Comandeer.
   4. Press Space
   5. Type https://ckeditor.com.
   6. Press Space

   Instead of two links with Comandeer between, we get https://cksource.com Comhttps://ckeditor.com. It's reproducible only in Blink and only if both links have the same beginnings (eg. http://cksource.com and http://ckeditor.com but not https://cksource.com and http://ckeditor.com as protocols don't match). Not sure if it isn't connected with fix for Mentions dropdown doesn't show up after activating styling #2038.

[Comandeer]

We traditionally use indent-based code blocks in our docs.

[jacekbogdanski]

Webkit issue was caused by moving selection at the end of the link ( which is not required on this browser ) and in the result, it created FILLING_CHAR_SEQUENCEat the end of the link element. Additional sequence was causing the issue with #2038 changes. However, these changes are not explicitly responsible for the issue and the easiest fix was ignoring selection change for Webkit browser, so #2038 is no blocker for this PR.

[Comandeer]

Heh, now I understand why there were 3 steps for autolink, not two… With the current version it's actually impossible to not autolink something on typing, e.g.

1. Type https://www.comandeer.pl
2. Press Space.
3. Press Ctrl/Cmd + Z.

Expected:

In Word undo removes link, but leaves space. Second undo removes everything (URL + space).

Actual:

In our implementation undo removes link with space. Adding that space also autolinks URL, so we're back to square one…

In both cases (three step and two step but without keeping space) our Undo behaves quite counter-intuitive. But, honestly, I don't know if there is any good solution to this issue (due to how our Undo works).

There is also one other issue with undo: after unlinking URL is selected. I'm not sure how to control selection in our Undo, but it's definitely worth further research.

[Comandeer]

The code here is nearly the same as in afterPaste handler above. I'd try to move it into separate, dedicated function. Condition – the only part that differs – could be passed as parameter.

[Comandeer]

It could look something like that:

function autolink( editor, condition ) {
	var matched = CKEDITOR.plugins.textMatch.match( editor.getSelection().getRanges()[ 0 ], matchCallback );

		if ( !condition( matched ) ) {
			return;
		}

		insertLink( matched );
}

editor.on( 'paste', function( evt ) {
	// Pasted text may be encoded. Decode it so we will be able to compare its length with match.
	var text = CKEDITOR.tools.htmlDecode( evt.data.dataValue );

	// Attach afterPaste event here to avoid race condition between events.
	editor.once( 'afterPaste', function() {
		autolink( editor, function( matched ) {
			return matched && matched.text.length === text.length;
		} )
	} );
} );
[…]
editor.on( 'contentDom', function() {
	var commitKeystrokes = editor.config.autolink_commitKeystrokes || CKEDITOR.config.autolink_commitKeystrokes;
	editor.editable().on( 'keydown', function( evt ) {
		if ( CKEDITOR.tools.indexOf( commitKeystrokes, evt.data.getKey() ) == -1 ) {
			return;
		}
		
		autolink( editor, function( matched ) {
			return matched;
		} );
		// Handle event ASAP thus some plugins may change
		// editor selection or cancel keydown events e.g. wysiwygarea, enterkey.
	}, null, null, 0 );
} );

[jacekbogdanski]

I extracted #2197 (review) issues into separate tickets so they will be easy to track and don't block this feature. See #2221 and #2220.

[Comandeer]

We're lacking unit and manual tests for Blink issue.

[Comandeer]

Isn't it Blink only issue?

[jacekbogdanski]

Nope, it occurs also on Edge.

[jacekbogdanski]

The unit test for continuous typing is disabled for WebKit thus it will result in a false negative. Although I kept manual test for each browser so we will have some safety net for the issue.

[jacekbogdanski]

I added an additional manual test for filling char sequence Webkit issue. However, I wasn't able to add a unit test for this issue thus Webkit behaves differently when manually typing text inside the editor and when changing editors content using javascript. In manual testing Webkit doesn't extend anchors when typing, however inserting text using javascript changes anchors text node which gives invalid testing results.

The issue doesn't exist in normal feature path, it was caused by special treatment for Firefox and Edge where anchors are extended by typing, so I believe the manual test could be enough for this case.

[Comandeer]

There is one issue in WebKit, connected with the fix for filling char sequence issue:

1. Paste https://comandeer.pl.
2. Paste http://comandeer.pl (just after the first pasted link, without any space or selection change)

Only one link is created as the selection is still inside the first link.

[Comandeer]

This test should be ignored also in IE as our plugin does not run there.

[Comandeer]

We have dedicated event for key handling in editor, key.

[Comandeer]

I'm not sure if we should interfere with other plugins in such way. If something is trying to stop keyboard event propagation, then it has some important reason for doing it.

[jacekbogdanski]

I fixed most of the issues except Blink one which has been extracted into a separate issue. See #2239 and #2240 for more information.

[Comandeer]

Rebased onto latest major.

[Comandeer]

LGTM!