From ecb507985b4bcf6bcafed545b3f422df92118a14 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Wed, 31 Jan 2018 13:22:50 +0100 Subject: [PATCH 01/14] Add unit test. --- tests/plugins/imagebase/features/caption.js | 27 ++++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/plugins/imagebase/features/caption.js b/tests/plugins/imagebase/features/caption.js index 8eced70b0f5..4a5738831df 100644 --- a/tests/plugins/imagebase/features/caption.js +++ b/tests/plugins/imagebase/features/caption.js @@ -1,7 +1,7 @@ /* bender-tags: editor,widget */ -/* bender-ckeditor-plugins: imagebase,toolbar,easyimage */ -/* bender-include: ../../widget/_helpers/tools.js */ -/* global widgetTestsTools */ +/* bender-ckeditor-plugins: imagebase,basicstyles,toolbar,easyimage */ +/* bender-include: ../../widget/_helpers/tools.js, %BASE_PATH%/plugins/easyimage/_helpers/tools.js */ +/* global widgetTestsTools, easyImageTools */ ( function() { 'use strict'; @@ -443,6 +443,7 @@ } } ), + // (#1776) 'test empty caption placeholder is hidden when blurred': function( editor, bot ) { addTestWidget( editor ); @@ -465,7 +466,25 @@ blurHost: emptyCaptionWidget } ); } ); - } + }, + + // #tp3384 + 'test caption placeholder integration with basicstyles': createToggleTest( { + fixture: 'toggleOneEmpty', + initial: false, + focus: true, + blur: false, + + customFocus: function( widget ) { + widget.focus(); + widget.parts.caption.focus(); + widget.editor.execCommand( 'bold' ); + }, + + onFocus: function( widget ) { + assertPlaceholder( widget, false ); + } + } ) }; tests = bender.tools.createTestsForEditors( CKEDITOR.tools.object.keys( bender.editors ), tests ); From 53c74267745c804aeeadfe14bc0cd89a2d49390c Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Wed, 31 Jan 2018 13:23:29 +0100 Subject: [PATCH 02/14] Add manual test. --- .../features/manual/captionbasicstyles.html | 51 +++++++++++++++++++ .../features/manual/captionbasicstyles.md | 24 +++++++++ 2 files changed, 75 insertions(+) create mode 100644 tests/plugins/imagebase/features/manual/captionbasicstyles.html create mode 100644 tests/plugins/imagebase/features/manual/captionbasicstyles.md diff --git a/tests/plugins/imagebase/features/manual/captionbasicstyles.html b/tests/plugins/imagebase/features/manual/captionbasicstyles.html new file mode 100644 index 00000000000..cc3dddfe4e7 --- /dev/null +++ b/tests/plugins/imagebase/features/manual/captionbasicstyles.html @@ -0,0 +1,51 @@ + + + + +

Classic editor

+ +
+

Widget without caption:

+
+ foo +
+ +

Widget with caption:

+
+ foo +
Test caption
+
+
+ +

Divarea

+ +
+

Widget without caption:

+
+ foo +
+ +

Widget with caption:

+
+ foo +
Test caption
+
+
+ + + diff --git a/tests/plugins/imagebase/features/manual/captionbasicstyles.md b/tests/plugins/imagebase/features/manual/captionbasicstyles.md new file mode 100644 index 00000000000..44fda1b4c64 --- /dev/null +++ b/tests/plugins/imagebase/features/manual/captionbasicstyles.md @@ -0,0 +1,24 @@ +@bender-tags: 4.9.0, feature, 932, tp3384 +@bender-ui: collapsed +@bender-ckeditor-plugins: sourcearea, wysiwygarea, toolbar, imagebase, link, htmlwriter, elementspath, basicstyles + +# Integration of caption and basicstyles + +1. Focus widget. +2. Put cursor inside the caption. If caption contains text, delete it. +3. Apply "Bold". + +## Expected + +* Bold is applied. +* There is no visible placeholder. +* Firefox/Edge: Selection remains inside the caption. + +## Unexpected + +* Placeholder shows up. +* Firefox/Edge: Selection is lost. + +--- + +Repeat for all editors. From a42106f12a69481a47ce369c428694faf0938916 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Wed, 31 Jan 2018 13:24:28 +0100 Subject: [PATCH 03/14] Check if focus is inside caption or element inside caption. --- plugins/imagebase/plugin.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/imagebase/plugin.js b/plugins/imagebase/plugin.js index fa4f86ba9d1..b6b2eb26728 100644 --- a/plugins/imagebase/plugin.js +++ b/plugins/imagebase/plugin.js @@ -599,10 +599,14 @@ caption = this.parts.caption, editable = this.editables.caption; + function isInCaption( element ) { + return element.equals( caption ) || caption.contains( element ); + } + if ( isFocused ) { - if ( !editable.getData() && !sender.equals( caption ) ) { + if ( !editable.getData() && !isInCaption( sender ) ) { addPlaceholder( this ); - } else if ( !sender || ( sender.equals( caption ) && sender.data( 'cke-caption-placeholder' ) ) ) { + } else if ( !sender || ( isInCaption( sender ) && caption.data( 'cke-caption-placeholder' ) ) ) { removePlaceholder( this ); } From 26d1af9d94125b18581144ed888b474ca39c05c2 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Wed, 31 Jan 2018 17:48:01 +0100 Subject: [PATCH 04/14] Introduce mechanism for forcing widget to be treated as focused. --- plugins/imagebase/plugin.js | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/plugins/imagebase/plugin.js b/plugins/imagebase/plugin.js index b6b2eb26728..e1e68c834df 100644 --- a/plugins/imagebase/plugin.js +++ b/plugins/imagebase/plugin.js @@ -554,7 +554,8 @@ function listener( evt ) { var path = evt.name === 'blur' ? editor.elementPath() : evt.data.path, sender = path ? path.lastElement : null, - widgets = getWidgetsWithFeature( editor.widgets.instances, 'caption' ); + widgets = getWidgetsWithFeature( editor.widgets.instances, 'caption' ), + focused = getFocusedWidget( editor ); if ( !editor.filter.check( 'figcaption' ) ) { return CKEDITOR.tools.array.forEach( listeners, function( listener ) { @@ -562,8 +563,16 @@ } ); } + // In Firefox and Edge applying styles inside caption results + // in firing this listener without focused widget. + // However it could be obtained from element, on which the event took place. + if ( editor.focusManager.hasFocus && sender && !focused ) { + focused = editor.widgets.getByElement( sender ); + } + + CKEDITOR.tools.array.forEach( widgets, function( widget ) { - widget._refreshCaption( sender ); + widget._refreshCaption( sender, widget === focused ); } ); } @@ -593,12 +602,18 @@ * @private * @member CKEDITOR.plugins.imagebase.featuresDefinitions.caption * @param {CKEDITOR.dom.element} sender The element that this function should be called on. + * @param {Boolean} [isFocused] Indicates if current widget should be treated as a focused one. + * If this parameter is omitted, its value is determined by checking + * {@link CKEDITOR.plugins.widget.repository#focused} value. */ - _refreshCaption: function( sender ) { - var isFocused = getFocusedWidget( this.editor ) === this, - caption = this.parts.caption, + _refreshCaption: function( sender, isFocused ) { + var caption = this.parts.caption, editable = this.editables.caption; + if ( typeof isFocused !== 'boolean' ) { + isFocused = getFocusedWidget( this.editor ) === this; + } + function isInCaption( element ) { return element.equals( caption ) || caption.contains( element ); } From b061847bc059c37aa44f58f23978287ded917e91 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Fri, 16 Feb 2018 16:34:12 +0100 Subject: [PATCH 05/14] Update manual test. --- .../plugins/imagebase/features/manual/captionbasicstyles.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/plugins/imagebase/features/manual/captionbasicstyles.md b/tests/plugins/imagebase/features/manual/captionbasicstyles.md index 44fda1b4c64..bc9401993e9 100644 --- a/tests/plugins/imagebase/features/manual/captionbasicstyles.md +++ b/tests/plugins/imagebase/features/manual/captionbasicstyles.md @@ -1,6 +1,6 @@ -@bender-tags: 4.9.0, feature, 932, tp3384 +@bender-tags: 4.9.1, feature, 932, tp3384, 1646 @bender-ui: collapsed -@bender-ckeditor-plugins: sourcearea, wysiwygarea, toolbar, imagebase, link, htmlwriter, elementspath, basicstyles +@bender-ckeditor-plugins: wysiwygarea, toolbar, imagebase, basicstyles # Integration of caption and basicstyles @@ -12,7 +12,7 @@ * Bold is applied. * There is no visible placeholder. -* Firefox/Edge: Selection remains inside the caption. +* Selection remains inside the caption. ## Unexpected From 476c5e28e2025e8f68a04aa6d73d768b9d0176e8 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Fri, 16 Feb 2018 16:35:45 +0100 Subject: [PATCH 06/14] Update API docs and issue references in code. --- plugins/imagebase/plugin.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/imagebase/plugin.js b/plugins/imagebase/plugin.js index e1e68c834df..6c2f8dd208a 100644 --- a/plugins/imagebase/plugin.js +++ b/plugins/imagebase/plugin.js @@ -565,7 +565,7 @@ // In Firefox and Edge applying styles inside caption results // in firing this listener without focused widget. - // However it could be obtained from element, on which the event took place. + // However it could be obtained from element, on which the event took place (#1646). if ( editor.focusManager.hasFocus && sender && !focused ) { focused = editor.widgets.getByElement( sender ); } @@ -604,7 +604,7 @@ * @param {CKEDITOR.dom.element} sender The element that this function should be called on. * @param {Boolean} [isFocused] Indicates if current widget should be treated as a focused one. * If this parameter is omitted, its value is determined by checking - * {@link CKEDITOR.plugins.widget.repository#focused} value. + * {@link CKEDITOR.plugins.widget.repository#focused} value. This parameter was added in 4.9.1. */ _refreshCaption: function( sender, isFocused ) { var caption = this.parts.caption, From a2d31b7fa537a181e31d248132d44b77d2d73e76 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Fri, 16 Feb 2018 17:35:39 +0100 Subject: [PATCH 07/14] Update unit test. --- tests/plugins/imagebase/features/caption.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/plugins/imagebase/features/caption.js b/tests/plugins/imagebase/features/caption.js index 4a5738831df..cf5584aa0a4 100644 --- a/tests/plugins/imagebase/features/caption.js +++ b/tests/plugins/imagebase/features/caption.js @@ -443,7 +443,6 @@ } } ), - // (#1776) 'test empty caption placeholder is hidden when blurred': function( editor, bot ) { addTestWidget( editor ); @@ -468,7 +467,7 @@ } ); }, - // #tp3384 + // #1646 'test caption placeholder integration with basicstyles': createToggleTest( { fixture: 'toggleOneEmpty', initial: false, @@ -476,8 +475,17 @@ blur: false, customFocus: function( widget ) { + var range = widget.editor.createRange(), + caption = widget.parts.caption; + widget.focus(); - widget.parts.caption.focus(); + caption.focus(); + + // In Safari and IE11 focus is not enough to move selection. + range.setStart( caption.getChild( 0 ), 0 ); + range.collapse(); + range.select(); + widget.editor.execCommand( 'bold' ); }, From 07df76964b590b9822c56475a445918fe115fa20 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Sat, 3 Mar 2018 14:06:12 +0100 Subject: [PATCH 08/14] Fix incorrect issue reference. --- tests/plugins/imagebase/features/caption.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/plugins/imagebase/features/caption.js b/tests/plugins/imagebase/features/caption.js index cf5584aa0a4..636398638b1 100644 --- a/tests/plugins/imagebase/features/caption.js +++ b/tests/plugins/imagebase/features/caption.js @@ -467,7 +467,7 @@ } ); }, - // #1646 + // (#1646) 'test caption placeholder integration with basicstyles': createToggleTest( { fixture: 'toggleOneEmpty', initial: false, From feb8bcab377855a92941c8fe7bbe0be707a8b453 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Sat, 3 Mar 2018 14:11:52 +0100 Subject: [PATCH 09/14] Ignore manual test in unsupported environments. --- .../plugins/imagebase/features/manual/captionbasicstyles.html | 4 ++++ tests/plugins/imagebase/features/manual/captionbasicstyles.md | 1 + 2 files changed, 5 insertions(+) diff --git a/tests/plugins/imagebase/features/manual/captionbasicstyles.html b/tests/plugins/imagebase/features/manual/captionbasicstyles.html index cc3dddfe4e7..6f44bb2bb37 100644 --- a/tests/plugins/imagebase/features/manual/captionbasicstyles.html +++ b/tests/plugins/imagebase/features/manual/captionbasicstyles.html @@ -33,6 +33,10 @@

Divarea