From 6fa23c96978e877d60b020cbd2a55074966c9714 Mon Sep 17 00:00:00 2001 From: Arkadiusz Filipczak Date: Wed, 20 Oct 2021 13:15:22 +0200 Subject: [PATCH 1/5] Link following: implementation --- packages/ckeditor5-link/src/linkediting.js | 47 +++++++++++++++++++++- packages/ckeditor5-link/src/utils.js | 11 +++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-link/src/linkediting.js b/packages/ckeditor5-link/src/linkediting.js index a6ccb4be84d..339700ac84e 100644 --- a/packages/ckeditor5-link/src/linkediting.js +++ b/packages/ckeditor5-link/src/linkediting.js @@ -11,12 +11,12 @@ import { Plugin } from 'ckeditor5/src/core'; import { MouseObserver } from 'ckeditor5/src/engine'; import { Input, TwoStepCaretMovement, inlineHighlight, findAttributeRange } from 'ckeditor5/src/typing'; import { ClipboardPipeline } from 'ckeditor5/src/clipboard'; -import { keyCodes } from 'ckeditor5/src/utils'; +import { keyCodes, env } from 'ckeditor5/src/utils'; import LinkCommand from './linkcommand'; import UnlinkCommand from './unlinkcommand'; import ManualDecorator from './utils/manualdecorator'; -import { createLinkElement, ensureSafeUrl, getLocalizedDecorators, normalizeDecorators } from './utils'; +import { createLinkElement, ensureSafeUrl, getLocalizedDecorators, normalizeDecorators, openLink } from './utils'; import '../theme/link.css'; @@ -107,6 +107,9 @@ export default class LinkEditing extends Plugin { // Setup highlight over selected link. inlineHighlight( editor, 'linkHref', 'a', HIGHLIGHT_CLASS ); + // Handle link following by CTRL+click or ALT+ENTER + this._enableLinkOpen(); + // Change the attributes of the selection in certain situations after the link was inserted into the document. this._enableInsertContentSelectionAttributesFixer(); @@ -220,6 +223,46 @@ export default class LinkEditing extends Plugin { } ); } + /** + * Attaches handlers for {@link module:engine/view/document~Document#event:enter} and + * {@link module:engine/view/document~Document#event:click} to enable link following. + * + * @private + */ + _enableLinkOpen() { + const editor = this.editor; + const view = editor.editing.view; + const viewDocument = view.document; + const modelDocument = editor.model.document; + + this.listenTo( viewDocument, 'click', ( evt, data ) => { + const shouldOpen = env.isMac ? data.domEvent.metaKey : data.domEvent.ctrlKey; + + if ( !shouldOpen ) { + return; + } + + const clickedElement = data.domTarget; + const url = clickedElement.getAttribute( 'href' ); + + evt.stop(); + openLink( url ); + }, { context: 'a' } ); + + this.listenTo( viewDocument, 'enter', ( evt, data ) => { + const selection = modelDocument.selection; + const url = selection.getAttribute( 'linkHref' ); + const shouldOpen = url && data.domEvent.altKey; + + if ( !shouldOpen ) { + return; + } + + evt.stop(); + openLink( url ); + }, { context: 'a' } ); + } + /** * Starts listening to {@link module:engine/model/model~Model#event:insertContent} and corrects the model * selection attributes if the selection is at the end of a link after inserting the content. diff --git a/packages/ckeditor5-link/src/utils.js b/packages/ckeditor5-link/src/utils.js index 3d668ba1ef3..de0e1e54b74 100644 --- a/packages/ckeditor5-link/src/utils.js +++ b/packages/ckeditor5-link/src/utils.js @@ -7,6 +7,8 @@ * @module link/utils */ +/* global window */ + import { upperFirst } from 'lodash-es'; const ATTRIBUTE_WHITESPACES = /[\u0000-\u0020\u00A0\u1680\u180E\u2000-\u2029\u205f\u3000]/g; // eslint-disable-line no-control-regex @@ -171,3 +173,12 @@ export function addLinkProtocolIfApplicable( link, defaultProtocol ) { return link && isProtocolNeeded ? protocol + link : link; } + +/** + * Opens the link in a new browser tab. + * + * @param {String} link + */ +export function openLink( link ) { + window.open( link, '_blank', 'noopener' ); +} From c5f654bd3dbe8671a5fc6b4d07746435ba0e2ed8 Mon Sep 17 00:00:00 2001 From: Arkadiusz Filipczak Date: Thu, 21 Oct 2021 15:40:56 +0200 Subject: [PATCH 2/5] Link following: tests --- packages/ckeditor5-link/tests/linkediting.js | 164 ++++++++++++++++++- packages/ckeditor5-link/tests/utils.js | 29 +++- 2 files changed, 191 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-link/tests/linkediting.js b/packages/ckeditor5-link/tests/linkediting.js index e6d5521a08f..00d65de8d0a 100644 --- a/packages/ckeditor5-link/tests/linkediting.js +++ b/packages/ckeditor5-link/tests/linkediting.js @@ -23,10 +23,11 @@ import { getData as getModelData, setData as setModelData } from '@ckeditor/cked import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import { isLinkElement } from '../src/utils'; +import { env } from 'ckeditor5/src/utils'; import '@ckeditor/ckeditor5-core/tests/_utils/assertions/attribute'; -/* global document */ +/* global document, window */ describe( 'LinkEditing', () => { let element, editor, model, view; @@ -940,6 +941,167 @@ describe( 'LinkEditing', () => { } ); } ); + describe( 'link following', () => { + let stub; + + beforeEach( () => { + stub = sinon.stub( window, 'open' ); + + stub.returns( undefined ); + } ); + + afterEach( () => { + stub.restore(); + } ); + + describe( 'using mouse', () => { + const initialEnvMac = env.isMac; + + afterEach( () => { + env.isMac = initialEnvMac; + } ); + + describe( 'on Mac', () => { + beforeEach( () => { + env.isMac = true; + } ); + + it( 'should follow the link after CMD+click', async () => { + setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); + + fireClickEvent( { metaKey: true, ctrlKey: false } ); + + expect( stub.calledOnce ).to.be.true; + expect( stub.calledOn( window ) ).to.be.true; + expect( stub.calledWith( 'http://www.ckeditor.com', '_blank', 'noopener' ) ).to.be.true; + } ); + + it( 'should not follow the link after CTRL+click', async () => { + setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); + + fireClickEvent( { metaKey: false, ctrlKey: true } ); + + expect( stub.notCalled ).to.be.true; + } ); + + it( 'should not follow the link after click with neither CMD nor CTRL pressed', async () => { + setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); + + fireClickEvent( { metaKey: false, ctrlKey: false } ); + + expect( stub.notCalled ).to.be.true; + } ); + } ); + + describe( 'on non-Mac', () => { + beforeEach( () => { + env.isMac = false; + } ); + + it( 'should follow the link after CTRL+click', async () => { + setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); + + fireClickEvent( { metaKey: false, ctrlKey: true } ); + + expect( stub.calledOnce ).to.be.true; + expect( stub.calledOn( window ) ).to.be.true; + expect( stub.calledWith( 'http://www.ckeditor.com', '_blank', 'noopener' ) ).to.be.true; + } ); + + it( 'should not follow the link after CMD+click', async () => { + setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); + + fireClickEvent( { metaKey: true, ctrlKey: false } ); + + expect( stub.notCalled ).to.be.true; + } ); + + it( 'should not follow the link after click with neither CMD nor CTRL pressed', async () => { + setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); + + fireClickEvent( { metaKey: false, ctrlKey: false } ); + + expect( stub.notCalled ).to.be.true; + } ); + } ); + + function fireClickEvent( options ) { + const linkElement = editor.ui.getEditableElement().getElementsByTagName( 'a' )[ 0 ]; + + view.document.fire( 'click', { + domTarget: linkElement, + domEvent: options, + preventDefault: () => {} + } ); + } + } ); + + describe( 'using keyboard', () => { + const positiveScenarios = [ + { + condition: 'selection is collapsed inside the link', + modelData: '<$text linkHref="http://www.ckeditor.com">Ba[]r' + }, + { + condition: 'selection is collapsed at the end of the link', + modelData: '<$text linkHref="http://www.ckeditor.com">Bar[]' + }, + { + condition: 'selection is collapsed at the begining of the link', + modelData: '<$text linkHref="http://www.ckeditor.com">[]Bar' + }, + { + condition: 'part of the link is selected', + modelData: '<$text linkHref="http://www.ckeditor.com">B[a]r' + }, + { + condition: 'the whole link is selected', + modelData: '<$text linkHref="http://www.ckeditor.com">B[a]r' + } + ]; + + for ( const { condition, modelData } of positiveScenarios ) { + it( `should open link after pressing ALT+ENTER if ${ condition }`, () => { + setModelData( model, modelData ); + + fireEnterPressedEvent( { altKey: true } ); + + expect( stub.calledOnce ).to.be.true; + expect( stub.calledOn( window ) ).to.be.true; + expect( stub.calledWith( 'http://www.ckeditor.com', '_blank', 'noopener' ) ).to.be.true; + } ); + } + + it( 'should not open link after pressing ENTER without ALT', () => { + setModelData( model, '<$text linkHref="http://www.ckeditor.com">Ba[]r' ); + + fireEnterPressedEvent( { altKey: false } ); + + expect( stub.notCalled ).to.be.true; + } ); + + it( 'should not open link after pressing ALT+ENTER if not inside a link', () => { + setModelData( model, '<$text linkHref="http://www.ckeditor.com">BarBaz[]' ); + + fireEnterPressedEvent( { altKey: true } ); + + expect( stub.notCalled ).to.be.true; + } ); + + function fireEnterPressedEvent( options ) { + view.document.fire( 'keydown', { + keyCode: keyCodes.enter, + domEvent: { + keyCode: keyCodes.enter, + preventDefault: () => {}, + target: document.body, + ...options + } + } ); + } + } ); + } ); + // https://github.com/ckeditor/ckeditor5/issues/1016 describe( 'typing around the link after a click', () => { let editor; diff --git a/packages/ckeditor5-link/tests/utils.js b/packages/ckeditor5-link/tests/utils.js index 6eaaac4afba..6583a0a8b9b 100644 --- a/packages/ckeditor5-link/tests/utils.js +++ b/packages/ckeditor5-link/tests/utils.js @@ -3,6 +3,8 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ +/* global window */ + import ViewDocument from '@ckeditor/ckeditor5-engine/src/view/document'; import ViewDowncastWriter from '@ckeditor/ckeditor5-engine/src/view/downcastwriter'; import AttributeElement from '@ckeditor/ckeditor5-engine/src/view/attributeelement'; @@ -17,7 +19,8 @@ import { normalizeDecorators, isLinkableElement, isEmail, - addLinkProtocolIfApplicable + addLinkProtocolIfApplicable, + openLink } from '../src/utils'; describe( 'utils', () => { @@ -303,4 +306,28 @@ describe( 'utils', () => { expect( addLinkProtocolIfApplicable( 'mailto:foo@bar.com', 'http://' ) ).to.equal( 'mailto:foo@bar.com' ); } ); } ); + + describe( 'openLink()', () => { + let stub; + + beforeEach( () => { + stub = sinon.stub( window, 'open' ); + + stub.returns( undefined ); + } ); + + afterEach( () => { + stub.restore(); + } ); + + it( 'should open a new browser tab', () => { + const url = 'http://www.ckeditor.com'; + + openLink( url ); + + expect( stub.calledOnce ).to.be.true; + expect( stub.calledOn( window ) ).to.be.true; + expect( stub.calledWith( url, '_blank', 'noopener' ) ).to.be.true; + } ); + } ); } ); From 1e84e066dd8a941020dfe0f3070850c07f80713f Mon Sep 17 00:00:00 2001 From: Arkadiusz Filipczak Date: Thu, 21 Oct 2021 15:47:44 +0200 Subject: [PATCH 3/5] Link following: typo fix --- packages/ckeditor5-link/tests/linkediting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-link/tests/linkediting.js b/packages/ckeditor5-link/tests/linkediting.js index 00d65de8d0a..e6c672e8188 100644 --- a/packages/ckeditor5-link/tests/linkediting.js +++ b/packages/ckeditor5-link/tests/linkediting.js @@ -1056,7 +1056,7 @@ describe( 'LinkEditing', () => { }, { condition: 'the whole link is selected', - modelData: '<$text linkHref="http://www.ckeditor.com">B[a]r' + modelData: '<$text linkHref="http://www.ckeditor.com">[Bar]' } ]; From e3f532e2b376bdc77a30fb4a9315d396558ed1a5 Mon Sep 17 00:00:00 2001 From: Arkadiusz Filipczak Date: Tue, 26 Oct 2021 16:20:47 +0200 Subject: [PATCH 4/5] Link following: refactor & tests --- packages/ckeditor5-link/src/linkediting.js | 28 +++++++- packages/ckeditor5-link/tests/linkediting.js | 71 ++++++++++++++++--- packages/ckeditor5-link/tests/linkimageui.js | 2 +- .../tests/mention-integration.js | 2 +- 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/packages/ckeditor5-link/src/linkediting.js b/packages/ckeditor5-link/src/linkediting.js index 339700ac84e..2dea53196c6 100644 --- a/packages/ckeditor5-link/src/linkediting.js +++ b/packages/ckeditor5-link/src/linkediting.js @@ -242,16 +242,37 @@ export default class LinkEditing extends Plugin { return; } - const clickedElement = data.domTarget; + let clickedElement = data.domTarget; + + if ( clickedElement.tagName.toLowerCase() != 'a' ) { + clickedElement = clickedElement.closest( 'a' ); + } + + if ( !clickedElement ) { + return; + } + const url = clickedElement.getAttribute( 'href' ); + if ( !url ) { + return; + } + evt.stop(); + data.preventDefault(); + openLink( url ); - }, { context: 'a' } ); + }, { context: '$capture' } ); this.listenTo( viewDocument, 'enter', ( evt, data ) => { const selection = modelDocument.selection; - const url = selection.getAttribute( 'linkHref' ); + + const selectedElement = selection.getSelectedElement(); + + const url = selectedElement ? + selectedElement.getAttribute( 'linkHref' ) : + selection.getAttribute( 'linkHref' ); + const shouldOpen = url && data.domEvent.altKey; if ( !shouldOpen ) { @@ -259,6 +280,7 @@ export default class LinkEditing extends Plugin { } evt.stop(); + openLink( url ); }, { context: 'a' } ); } diff --git a/packages/ckeditor5-link/tests/linkediting.js b/packages/ckeditor5-link/tests/linkediting.js index e6c672e8188..82d06349920 100644 --- a/packages/ckeditor5-link/tests/linkediting.js +++ b/packages/ckeditor5-link/tests/linkediting.js @@ -19,6 +19,7 @@ import ImageBlockEditing from '@ckeditor/ckeditor5-image/src/image/imageblockedi import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Input from '@ckeditor/ckeditor5-typing/src/input'; import Delete from '@ckeditor/ckeditor5-typing/src/delete'; +import ImageInline from '@ckeditor/ckeditor5-image/src/imageinline'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; @@ -37,7 +38,7 @@ describe( 'LinkEditing', () => { document.body.appendChild( element ); editor = await ClassicTestEditor.create( element, { - plugins: [ Paragraph, LinkEditing, Enter, Clipboard ], + plugins: [ Paragraph, LinkEditing, Enter, Clipboard, ImageInline ], link: { decorators: { isExternal: { @@ -942,7 +943,7 @@ describe( 'LinkEditing', () => { } ); describe( 'link following', () => { - let stub; + let stub, eventPreventDefault; beforeEach( () => { stub = sinon.stub( window, 'open' ); @@ -966,7 +967,7 @@ describe( 'LinkEditing', () => { env.isMac = true; } ); - it( 'should follow the link after CMD+click', async () => { + it( 'should follow the link after CMD+click', () => { setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); fireClickEvent( { metaKey: true, ctrlKey: false } ); @@ -974,22 +975,25 @@ describe( 'LinkEditing', () => { expect( stub.calledOnce ).to.be.true; expect( stub.calledOn( window ) ).to.be.true; expect( stub.calledWith( 'http://www.ckeditor.com', '_blank', 'noopener' ) ).to.be.true; + expect( eventPreventDefault.calledOnce ).to.be.true; } ); - it( 'should not follow the link after CTRL+click', async () => { + it( 'should not follow the link after CTRL+click', () => { setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); fireClickEvent( { metaKey: false, ctrlKey: true } ); expect( stub.notCalled ).to.be.true; + expect( eventPreventDefault.calledOnce ).to.be.false; } ); - it( 'should not follow the link after click with neither CMD nor CTRL pressed', async () => { + it( 'should not follow the link after click with neither CMD nor CTRL pressed', () => { setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); fireClickEvent( { metaKey: false, ctrlKey: false } ); expect( stub.notCalled ).to.be.true; + expect( eventPreventDefault.calledOnce ).to.be.false; } ); } ); @@ -998,7 +1002,7 @@ describe( 'LinkEditing', () => { env.isMac = false; } ); - it( 'should follow the link after CTRL+click', async () => { + it( 'should follow the link after CTRL+click', () => { setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); fireClickEvent( { metaKey: false, ctrlKey: true } ); @@ -1008,7 +1012,7 @@ describe( 'LinkEditing', () => { expect( stub.calledWith( 'http://www.ckeditor.com', '_blank', 'noopener' ) ).to.be.true; } ); - it( 'should not follow the link after CMD+click', async () => { + it( 'should not follow the link after CMD+click', () => { setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); fireClickEvent( { metaKey: true, ctrlKey: false } ); @@ -1016,7 +1020,7 @@ describe( 'LinkEditing', () => { expect( stub.notCalled ).to.be.true; } ); - it( 'should not follow the link after click with neither CMD nor CTRL pressed', async () => { + it( 'should not follow the link after click with neither CMD nor CTRL pressed', () => { setModelData( model, '<$text linkHref="http://www.ckeditor.com">Bar[]' ); fireClickEvent( { metaKey: false, ctrlKey: false } ); @@ -1025,13 +1029,54 @@ describe( 'LinkEditing', () => { } ); } ); - function fireClickEvent( options ) { - const linkElement = editor.ui.getEditableElement().getElementsByTagName( 'a' )[ 0 ]; + it( 'should follow the inline image link', () => { + setModelData( model, '[]' ); + + fireClickEvent( { metaKey: env.isMac, ctrlKey: !env.isMac }, 'img' ); + + expect( stub.calledOnce ).to.be.true; + expect( stub.calledOn( window ) ).to.be.true; + expect( stub.calledWith( 'http://www.ckeditor.com', '_blank', 'noopener' ) ).to.be.true; + expect( eventPreventDefault.calledOnce ).to.be.true; + } ); + + it( 'should now follow the link if "a" element doesn\'t have "href" attribute', () => { + editor.conversion.attributeToElement( { + model: 'customLink', + view: 'a' + } ); + + setModelData( model, '<$text customLink="">Bar[]' ); + + fireClickEvent( { metaKey: env.isMac, ctrlKey: !env.isMac } ); + + expect( stub.notCalled ).to.be.true; + expect( eventPreventDefault.calledOnce ).to.be.false; + } ); + + it( 'should not follow the link if no link is clicked', () => { + editor.conversion.attributeToElement( { + model: 'customLink', + view: 'span' + } ); + + setModelData( model, '<$text customLink="">Bar[]' ); + + fireClickEvent( { metaKey: env.isMac, ctrlKey: !env.isMac }, 'span' ); + + expect( stub.notCalled ).to.be.true; + expect( eventPreventDefault.calledOnce ).to.be.false; + } ); + + function fireClickEvent( options, tagName = 'a' ) { + const linkElement = editor.ui.getEditableElement().getElementsByTagName( tagName )[ 0 ]; + + eventPreventDefault = sinon.spy(); view.document.fire( 'click', { domTarget: linkElement, domEvent: options, - preventDefault: () => {} + preventDefault: eventPreventDefault } ); } } ); @@ -1057,6 +1102,10 @@ describe( 'LinkEditing', () => { { condition: 'the whole link is selected', modelData: '<$text linkHref="http://www.ckeditor.com">[Bar]' + }, + { + condition: 'linked image is selected', + modelData: '[]' } ]; diff --git a/packages/ckeditor5-link/tests/linkimageui.js b/packages/ckeditor5-link/tests/linkimageui.js index 3e1a6ee5e67..cfcb66c3140 100644 --- a/packages/ckeditor5-link/tests/linkimageui.js +++ b/packages/ckeditor5-link/tests/linkimageui.js @@ -59,7 +59,7 @@ describe( 'LinkImageUI', () => { listenToSpy( viewDocument, 'click' ); - viewDocument.fire( 'click' ); + viewDocument.fire( 'click', { domEvent: {} } ); sinon.assert.calledOnce( listenToSpy ); } ); diff --git a/packages/ckeditor5-mention/tests/mention-integration.js b/packages/ckeditor5-mention/tests/mention-integration.js index d8d678eccc5..762b8f42900 100644 --- a/packages/ckeditor5-mention/tests/mention-integration.js +++ b/packages/ckeditor5-mention/tests/mention-integration.js @@ -313,7 +313,7 @@ describe( 'Mention feature - integration', () => { writer.setSelection( writer.createRangeOn( model.document.getRoot().getChild( 0 ).getChild( 0 ) ) ); } ); - editor.editing.view.document.fire( 'click' ); + editor.editing.view.document.fire( 'click', { domEvent: {} } ); expect( panelView.isVisible ).to.be.true; expect( balloon.visibleView === mentionsView ).to.be.false; // LinkUI From 318bdc23988dcaaffa8d6f28c8dba6dcefef986c Mon Sep 17 00:00:00 2001 From: Kuba Niegowski <1232187+niegowski@users.noreply.github.com> Date: Tue, 2 Nov 2021 12:46:20 +0100 Subject: [PATCH 5/5] Apply review comment. --- packages/ckeditor5-link/tests/linkediting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-link/tests/linkediting.js b/packages/ckeditor5-link/tests/linkediting.js index 82d06349920..da079ee5a04 100644 --- a/packages/ckeditor5-link/tests/linkediting.js +++ b/packages/ckeditor5-link/tests/linkediting.js @@ -1040,7 +1040,7 @@ describe( 'LinkEditing', () => { expect( eventPreventDefault.calledOnce ).to.be.true; } ); - it( 'should now follow the link if "a" element doesn\'t have "href" attribute', () => { + it( 'should not follow the link if "a" element doesn\'t have "href" attribute', () => { editor.conversion.attributeToElement( { model: 'customLink', view: 'a'