From ac274b7fa00ad43f79a248e575810e59bfb565c9 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 9 Jul 2020 19:02:43 +0200 Subject: [PATCH 1/4] Widget type around UI should not be selectable by triple-click. --- .../theme/ckeditor5-widget/widgettypearound.css | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/ckeditor5-theme-lark/theme/ckeditor5-widget/widgettypearound.css b/packages/ckeditor5-theme-lark/theme/ckeditor5-widget/widgettypearound.css index 6f9b3c1c2a2..affed0ce585 100644 --- a/packages/ckeditor5-theme-lark/theme/ckeditor5-widget/widgettypearound.css +++ b/packages/ckeditor5-theme-lark/theme/ckeditor5-widget/widgettypearound.css @@ -24,6 +24,12 @@ } .ck .ck-widget { + /* + * Disable triple-click selection of UI elements. + */ + & .ck-widget__type-around { + user-select: none; + } /* * Styles of the type around buttons */ From 752b8dc373a63a94582f47f0df75eba44ba40c64 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 10 Jul 2020 13:03:33 +0200 Subject: [PATCH 2/4] Updated triple-click handler inside nested editable to include Gecko support and properly handle click on AttributeElement. --- .../ckeditor5-widget/widgettypearound.css | 6 ---- packages/ckeditor5-widget/package.json | 1 + packages/ckeditor5-widget/src/widget.js | 9 ++++-- .../tests/widget-integration.js | 28 ++++++++++++++++++- 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-theme-lark/theme/ckeditor5-widget/widgettypearound.css b/packages/ckeditor5-theme-lark/theme/ckeditor5-widget/widgettypearound.css index affed0ce585..6f9b3c1c2a2 100644 --- a/packages/ckeditor5-theme-lark/theme/ckeditor5-widget/widgettypearound.css +++ b/packages/ckeditor5-theme-lark/theme/ckeditor5-widget/widgettypearound.css @@ -24,12 +24,6 @@ } .ck .ck-widget { - /* - * Disable triple-click selection of UI elements. - */ - & .ck-widget__type-around { - user-select: none; - } /* * Styles of the type around buttons */ diff --git a/packages/ckeditor5-widget/package.json b/packages/ckeditor5-widget/package.json index 121e4e5d054..85634152b7b 100644 --- a/packages/ckeditor5-widget/package.json +++ b/packages/ckeditor5-widget/package.json @@ -27,6 +27,7 @@ "@ckeditor/ckeditor5-heading": "^20.0.0", "@ckeditor/ckeditor5-horizontal-line": "^20.0.0", "@ckeditor/ckeditor5-image": "^20.0.0", + "@ckeditor/ckeditor5-link": "^20.0.0", "@ckeditor/ckeditor5-media-embed": "^20.0.0", "@ckeditor/ckeditor5-paragraph": "^20.0.0", "@ckeditor/ckeditor5-table": "^20.0.0", diff --git a/packages/ckeditor5-widget/src/widget.js b/packages/ckeditor5-widget/src/widget.js index da035fefd04..d9475e3e5e3 100644 --- a/packages/ckeditor5-widget/src/widget.js +++ b/packages/ckeditor5-widget/src/widget.js @@ -146,12 +146,15 @@ export default class Widget extends Plugin { // But at least triple click inside nested editable causes broken selection in Safari. // For such event, we select the entire nested editable element. // See: https://github.com/ckeditor/ckeditor5/issues/1463. - if ( env.isSafari && domEventData.domEvent.detail >= 3 ) { + if ( ( env.isSafari || env.isGecko ) && domEventData.domEvent.detail >= 3 ) { const mapper = editor.editing.mapper; - const modelElement = mapper.toModelElement( element ); + const viewElement = element.is( 'attributeElement' ) ? + element.findAncestor( element => !element.is( 'attributeElement' ) ) : element; + const modelElement = mapper.toModelElement( viewElement ); + + domEventData.preventDefault(); this.editor.model.change( writer => { - domEventData.preventDefault(); writer.setSelection( modelElement, 'in' ); } ); } diff --git a/packages/ckeditor5-widget/tests/widget-integration.js b/packages/ckeditor5-widget/tests/widget-integration.js index 12d8f0e2cbd..f621e296901 100644 --- a/packages/ckeditor5-widget/tests/widget-integration.js +++ b/packages/ckeditor5-widget/tests/widget-integration.js @@ -8,6 +8,7 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Typing from '@ckeditor/ckeditor5-typing/src/typing'; +import LinkEditing from '@ckeditor/ckeditor5-link/src/linkediting'; import Widget from '../src/widget'; import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata'; @@ -32,7 +33,7 @@ describe( 'Widget - integration', () => { editorElement = document.createElement( 'div' ); document.body.appendChild( editorElement ); - return ClassicEditor.create( editorElement, { plugins: [ Paragraph, Widget, Typing ] } ) + return ClassicEditor.create( editorElement, { plugins: [ Paragraph, Widget, Typing, LinkEditing ] } ) .then( newEditor => { editor = newEditor; model = editor.model; @@ -148,6 +149,31 @@ describe( 'Widget - integration', () => { expect( getModelData( model ) ).to.equal( '[foo bar]' ); } ); + it( 'should select the entire nested editable if triple clicked on link', () => { + setModelData( model, '[]foo <$text linkHref="abc">bar' ); + + const viewDiv = viewDocument.getRoot().getChild( 0 ); + const viewLink = viewDiv.getChild( 0 ).getChild( 1 ); + const preventDefault = sinon.spy(); + const domEventDataMock = new DomEventData( view, { + target: view.domConverter.mapViewToDom( viewLink ), + preventDefault, + detail: 3 + } ); + + viewDocument.fire( 'mousedown', domEventDataMock ); + + sinon.assert.called( preventDefault ); + + expect( getViewData( view ) ).to.equal( + '
' + + '
{foo bar]
' + + '
' + + '
' + ); + expect( getModelData( model ) ).to.equal( '[foo <$text linkHref="abc">bar]' ); + } ); + it( 'should select proper nested editable if triple clicked', () => { setModelData( model, '[]foobar' ); From 390f127fc859d09ca714defaf9fb507f605980ad Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 10 Jul 2020 17:07:21 +0200 Subject: [PATCH 3/4] Added test for triple-click in nested editable with multiple paragraphs. --- .../tests/widget-integration.js | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/packages/ckeditor5-widget/tests/widget-integration.js b/packages/ckeditor5-widget/tests/widget-integration.js index f621e296901..a49b65a7369 100644 --- a/packages/ckeditor5-widget/tests/widget-integration.js +++ b/packages/ckeditor5-widget/tests/widget-integration.js @@ -59,6 +59,9 @@ describe( 'Widget - integration', () => { isObject: true, isInline: true } ); + model.schema.extend( '$block', { + allowIn: 'nested' + } ); editor.conversion.for( 'downcast' ) .elementToElement( { model: 'inline', view: 'figure' } ) @@ -174,6 +177,51 @@ describe( 'Widget - integration', () => { expect( getModelData( model ) ).to.equal( '[foo <$text linkHref="abc">bar]' ); } ); + it( 'should select only clicked paragraph if triple clicked on link', () => { + setModelData( model, + '[]' + + '' + + 'foo' + + 'foo <$text linkHref="abc">bar' + + 'bar' + + '' + + '' + ); + + const viewDiv = viewDocument.getRoot().getChild( 0 ); + const viewLink = viewDiv.getChild( 0 ).getChild( 1 ).getChild( 1 ); + const preventDefault = sinon.spy(); + const domEventDataMock = new DomEventData( view, { + target: view.domConverter.mapViewToDom( viewLink ), + preventDefault, + detail: 3 + } ); + + viewDocument.fire( 'mousedown', domEventDataMock ); + + sinon.assert.called( preventDefault ); + + expect( getViewData( view ) ).to.equal( + '
' + + '
' + + '

foo

' + + '

{foo bar]

' + + '

bar

' + + '
' + + '
' + + '
' + ); + expect( getModelData( model ) ).to.equal( + '' + + '' + + 'foo' + + '[foo <$text linkHref="abc">bar]' + + 'bar' + + '' + + '' + ); + } ); + it( 'should select proper nested editable if triple clicked', () => { setModelData( model, '[]foobar' ); From 96aa2e7d0ac9588333689a1e9d21043d8904266c Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 10 Jul 2020 17:19:54 +0200 Subject: [PATCH 4/4] Updated test for gecko browsers. --- packages/ckeditor5-widget/tests/widget-integration.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-widget/tests/widget-integration.js b/packages/ckeditor5-widget/tests/widget-integration.js index a49b65a7369..b065f66019d 100644 --- a/packages/ckeditor5-widget/tests/widget-integration.js +++ b/packages/ckeditor5-widget/tests/widget-integration.js @@ -298,8 +298,9 @@ describe( 'Widget - integration', () => { expect( getModelData( model ) ).to.equal( 'Foo[foo bar]Bar' ); } ); - it( 'should do nothing for non-Safari browser', () => { + it( 'should do nothing for non-Safari and non-Gecko browser', () => { testUtils.sinon.stub( env, 'isSafari' ).get( () => false ); + testUtils.sinon.stub( env, 'isGecko' ).get( () => false ); setModelData( model, '[]foo bar' );