From 215d0d4704afee017b2a11acea41e08eeca64f2f Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Wed, 6 Dec 2017 00:10:14 +0100 Subject: [PATCH 01/11] Added change listener to the Balloon Toolbar, so that content change also causes context manager to be refreshed. --- plugins/balloontoolbar/plugin.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/balloontoolbar/plugin.js b/plugins/balloontoolbar/plugin.js index cea6006043a..ee1aea373a7 100644 --- a/plugins/balloontoolbar/plugin.js +++ b/plugins/balloontoolbar/plugin.js @@ -601,6 +601,11 @@ var editable = this.editor.editable(); this._detachListeners(); + this._listeners.push( this.editor.on( 'change', function() { + this.attach( this._pointedElement, { + focusElement: false + } ); + }, this ) ); this._listeners.push( this.editor.on( 'resize', function() { this.attach( this._pointedElement, { focusElement: false From 31f7a7597d334571edbb3ff4c626bf4085341363 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Fri, 15 Dec 2017 16:07:41 +0100 Subject: [PATCH 02/11] Add unit test. --- .../plugins/balloontoolbar/refreshposition.js | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 tests/plugins/balloontoolbar/refreshposition.js diff --git a/tests/plugins/balloontoolbar/refreshposition.js b/tests/plugins/balloontoolbar/refreshposition.js new file mode 100644 index 00000000000..3369cbf232d --- /dev/null +++ b/tests/plugins/balloontoolbar/refreshposition.js @@ -0,0 +1,88 @@ +/* bender-tags: balloontoolbar */ +/* bender-ckeditor-plugins: toolbar,balloontoolbar,floatingspace,image2,undo */ +/* bender-include: _helpers/default.js */ +/* global ignoreUnsupportedEnvironment */ + +( function() { + 'use strict'; + + var commonConfig = { + allowedContent: true, + height: 200 + }; + + bender.editors = { + classic: { + config: commonConfig + }, + + divarea: { + config: CKEDITOR.tools.object.merge( commonConfig, { extraPlugins: 'divarea' } ) + }, + + inline: { + creator: 'inline', + config: commonConfig + } + }; + + var parentFrame = window.frameElement, + originalHeight = parentFrame && parentFrame.style.height; + + var tests = { + setUp: function() { + // In IE8 tests are run in very small window which breaks positioning assertions and tests fails (#1076). + if ( CKEDITOR.env.ie && CKEDITOR.env.version < 9 ) { + assert.ignore(); + } + if ( parentFrame ) { + parentFrame.style.height = '900px'; + } + + CKEDITOR.addCss( '.sideimage {' + + 'float: right;' + + 'width: 25%;' + + '}' ); + }, + + tearDown: function() { + if ( parentFrame ) { + parentFrame.style.height = originalHeight; + } + }, + + // #1342 + 'test panel refresh position': function( editor, bot ) { + + bot.setData( '
', function() { + var balloonToolbar = new CKEDITOR.ui.balloonToolbarView( editor, { + width: 100, + height: 200 + } ), + widget = editor.widgets.instances[ 0 ], + markerElement = widget.element, + initialPosition, + currentPosition; + + balloonToolbar.attach( markerElement ); + initialPosition = balloonToolbar.parts.panel.getClientRect(); + + editor.once( 'change', function() { + resume( function() { + currentPosition = balloonToolbar.parts.panel.getClientRect(); + assert.areNotSame( initialPosition.left, currentPosition.left, 'position of toolbar' ); + } ); + } ); + + markerElement.setStyle( 'margin-left', '200px' ); + editor.fire( 'change' ); + + wait(); + } ); + } + }; + + tests = bender.tools.createTestsForEditors( CKEDITOR.tools.objectKeys( bender.editors ), tests ); + ignoreUnsupportedEnvironment( tests ); + bender.test( tests ); +} )(); From 10d322976249f5c5709e32258810bae8731167de Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Fri, 15 Dec 2017 16:10:37 +0100 Subject: [PATCH 03/11] Add manual test. --- .../manual/refreshposition.html | 70 +++++++++++++++++++ .../balloontoolbar/manual/refreshposition.md | 17 +++++ 2 files changed, 87 insertions(+) create mode 100644 tests/plugins/balloontoolbar/manual/refreshposition.html create mode 100644 tests/plugins/balloontoolbar/manual/refreshposition.md diff --git a/tests/plugins/balloontoolbar/manual/refreshposition.html b/tests/plugins/balloontoolbar/manual/refreshposition.html new file mode 100644 index 00000000000..2dfb37e35b0 --- /dev/null +++ b/tests/plugins/balloontoolbar/manual/refreshposition.html @@ -0,0 +1,70 @@ + + +
+

This is a sample image widget:

+ +
+ Saturn V +
This is an example caption description
+
+
+ +
+

This is a sample image widget:

+ +
+ Saturn V +
This is an example caption description
+
+
+ + diff --git a/tests/plugins/balloontoolbar/manual/refreshposition.md b/tests/plugins/balloontoolbar/manual/refreshposition.md new file mode 100644 index 00000000000..014d877fc0b --- /dev/null +++ b/tests/plugins/balloontoolbar/manual/refreshposition.md @@ -0,0 +1,17 @@ +@bender-ui: collapsed +@bender-tags: 4.8.1, bug, balloontoolbar, 1342 +@bender-ckeditor-plugins: wysiwygarea, basicstyles, floatingspace, balloontoolbar, sourcearea, link, elementspath, image2, undo + +## Procedure + +1. Click image. +2. Wait 5 seconds for image to change position. +3. Look at balloon toolbar. + +## Expected: + +Balloon toolbar changes position acordingly to image position. + +## Unexpected: + +Balloon toolbar remains in the initial position. From b4c6d21cde8f8fe2707239c630ab020f00427f78 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Fri, 15 Dec 2017 16:21:08 +0100 Subject: [PATCH 04/11] Extract common listener code. --- plugins/balloontoolbar/plugin.js | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/plugins/balloontoolbar/plugin.js b/plugins/balloontoolbar/plugin.js index ee1aea373a7..86950d90bfc 100644 --- a/plugins/balloontoolbar/plugin.js +++ b/plugins/balloontoolbar/plugin.js @@ -601,26 +601,17 @@ var editable = this.editor.editable(); this._detachListeners(); - this._listeners.push( this.editor.on( 'change', function() { + function attachListener() { this.attach( this._pointedElement, { focusElement: false } ); - }, this ) ); - this._listeners.push( this.editor.on( 'resize', function() { - this.attach( this._pointedElement, { - focusElement: false - } ); - }, this ) ); - this._listeners.push( editable.attachListener( editable.getDocument(), 'scroll', function() { - this.attach( this._pointedElement, { - focusElement: false - } ); - }, this ) ); - this._listeners.push( CKEDITOR.document.getWindow().on( 'resize', function() { - this.attach( this._pointedElement, { - focusElement: false - } ); - }, this ) ); + } + + this._listeners.push( this.editor.on( 'change', attachListener, this ) ); + this._listeners.push( this.editor.on( 'resize', attachListener, this ) ); + this._listeners.push( editable.attachListener( editable.getDocument(), 'scroll', + attachListener, this ) ); + this._listeners.push( CKEDITOR.document.getWindow().on( 'resize', attachListener, this ) ); CKEDITOR.ui.balloonPanel.prototype.show.call( this ); }; From 8a3be4161ebc13308e756c1f133bce6d77f92015 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Fri, 15 Dec 2017 16:21:22 +0100 Subject: [PATCH 05/11] Update unit test. --- tests/plugins/balloontoolbar/view.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/plugins/balloontoolbar/view.js b/tests/plugins/balloontoolbar/view.js index 103de282e13..c44272614fa 100644 --- a/tests/plugins/balloontoolbar/view.js +++ b/tests/plugins/balloontoolbar/view.js @@ -78,7 +78,7 @@ view.show(); assert.isTrue( view.rect.visible, 'Toolbar should be shown after show method' ); - assert.areEqual( 3, view._listeners.length, 'Listeners should be attached after show method' ); + assert.areEqual( 4, view._listeners.length, 'Listeners should be attached after show method' ); view.hide(); assert.isFalse( view.rect.visible, 'Toolbar should not be shown after hide method' ); From 2186788ce01c3427c5d4d7bbdf5b5912c49e5714 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Fri, 15 Dec 2017 17:00:10 +0100 Subject: [PATCH 06/11] Adjusted listeners order. Both resize listeners seem to lay out better. In addition placed the document scroll listener in a single line, as it's not that long. --- plugins/balloontoolbar/plugin.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/balloontoolbar/plugin.js b/plugins/balloontoolbar/plugin.js index 86950d90bfc..bf347e9edad 100644 --- a/plugins/balloontoolbar/plugin.js +++ b/plugins/balloontoolbar/plugin.js @@ -609,9 +609,8 @@ this._listeners.push( this.editor.on( 'change', attachListener, this ) ); this._listeners.push( this.editor.on( 'resize', attachListener, this ) ); - this._listeners.push( editable.attachListener( editable.getDocument(), 'scroll', - attachListener, this ) ); this._listeners.push( CKEDITOR.document.getWindow().on( 'resize', attachListener, this ) ); + this._listeners.push( editable.attachListener( editable.getDocument(), 'scroll', attachListener, this ) ); CKEDITOR.ui.balloonPanel.prototype.show.call( this ); }; From 69d4d1b600f2dd4f2955150b07a7ef6082138978 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Mon, 18 Dec 2017 12:17:59 +0100 Subject: [PATCH 07/11] Shortened waiting time in manual test. --- tests/plugins/balloontoolbar/manual/refreshposition.html | 2 +- tests/plugins/balloontoolbar/manual/refreshposition.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/plugins/balloontoolbar/manual/refreshposition.html b/tests/plugins/balloontoolbar/manual/refreshposition.html index 2dfb37e35b0..6dcd90a9822 100644 --- a/tests/plugins/balloontoolbar/manual/refreshposition.html +++ b/tests/plugins/balloontoolbar/manual/refreshposition.html @@ -59,7 +59,7 @@ editor.widgets.instances[ 0 ].on( 'focus', function( evt ) { setTimeout( function() { editor.execCommand( 'sideImage', { widget: evt.sender } ); - }, 5000 ); + }, 2000 ); } ); } } diff --git a/tests/plugins/balloontoolbar/manual/refreshposition.md b/tests/plugins/balloontoolbar/manual/refreshposition.md index 014d877fc0b..3969403d9e2 100644 --- a/tests/plugins/balloontoolbar/manual/refreshposition.md +++ b/tests/plugins/balloontoolbar/manual/refreshposition.md @@ -5,7 +5,7 @@ ## Procedure 1. Click image. -2. Wait 5 seconds for image to change position. +2. Wait 2 seconds for image to change position. 3. Look at balloon toolbar. ## Expected: From ad73b1466af98246fb7707bb7984c71080c7f280 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Mon, 18 Dec 2017 12:25:01 +0100 Subject: [PATCH 08/11] Move unit test to positioning.js. --- tests/plugins/balloontoolbar/positioning.js | 33 ++++++- .../plugins/balloontoolbar/refreshposition.js | 88 ------------------- 2 files changed, 31 insertions(+), 90 deletions(-) delete mode 100644 tests/plugins/balloontoolbar/refreshposition.js diff --git a/tests/plugins/balloontoolbar/positioning.js b/tests/plugins/balloontoolbar/positioning.js index a240b9c31e6..7182731bf96 100644 --- a/tests/plugins/balloontoolbar/positioning.js +++ b/tests/plugins/balloontoolbar/positioning.js @@ -10,7 +10,7 @@ name: 'editor1', creator: 'replace', config: { - extraAllowedContent: 'span[id];p{height}', + extraAllowedContent: 'span[id];p{height};img[src]{margin-left}', height: 200 } }, @@ -19,7 +19,7 @@ creator: 'replace', config: { extraPlugins: 'divarea', - extraAllowedContent: 'span[id];p{height}', + extraAllowedContent: 'span[id];p{height};img[src]{margin-left}', height: 200 } } @@ -135,6 +135,35 @@ res = balloonToolbar._getAlignments( editor.editable().getFirst().getClientRect(), 10, 10 ); arrayAssert.itemsAreEqual( [ 'bottom hcenter', 'top hcenter' ], CKEDITOR.tools.objectKeys( res ) ); + }, + + // #1342 + 'test panel refresh position': function( editor, bot ) { + + bot.setData( '', function() { + var balloonToolbar = new CKEDITOR.ui.balloonToolbarView( editor, { + width: 100, + height: 200 + } ), + markerElement = editor.editable().findOne( 'img' ), + initialPosition, + currentPosition; + + balloonToolbar.attach( markerElement ); + initialPosition = balloonToolbar.parts.panel.getClientRect(); + + editor.once( 'change', function() { + resume( function() { + currentPosition = balloonToolbar.parts.panel.getClientRect(); + assert.areNotSame( initialPosition.left, currentPosition.left, 'position of toolbar' ); + } ); + } ); + + markerElement.setStyle( 'margin-left', '200px' ); + editor.fire( 'change' ); + + wait(); + } ); } }; diff --git a/tests/plugins/balloontoolbar/refreshposition.js b/tests/plugins/balloontoolbar/refreshposition.js deleted file mode 100644 index 3369cbf232d..00000000000 --- a/tests/plugins/balloontoolbar/refreshposition.js +++ /dev/null @@ -1,88 +0,0 @@ -/* bender-tags: balloontoolbar */ -/* bender-ckeditor-plugins: toolbar,balloontoolbar,floatingspace,image2,undo */ -/* bender-include: _helpers/default.js */ -/* global ignoreUnsupportedEnvironment */ - -( function() { - 'use strict'; - - var commonConfig = { - allowedContent: true, - height: 200 - }; - - bender.editors = { - classic: { - config: commonConfig - }, - - divarea: { - config: CKEDITOR.tools.object.merge( commonConfig, { extraPlugins: 'divarea' } ) - }, - - inline: { - creator: 'inline', - config: commonConfig - } - }; - - var parentFrame = window.frameElement, - originalHeight = parentFrame && parentFrame.style.height; - - var tests = { - setUp: function() { - // In IE8 tests are run in very small window which breaks positioning assertions and tests fails (#1076). - if ( CKEDITOR.env.ie && CKEDITOR.env.version < 9 ) { - assert.ignore(); - } - if ( parentFrame ) { - parentFrame.style.height = '900px'; - } - - CKEDITOR.addCss( '.sideimage {' + - 'float: right;' + - 'width: 25%;' + - '}' ); - }, - - tearDown: function() { - if ( parentFrame ) { - parentFrame.style.height = originalHeight; - } - }, - - // #1342 - 'test panel refresh position': function( editor, bot ) { - - bot.setData( '
', function() { - var balloonToolbar = new CKEDITOR.ui.balloonToolbarView( editor, { - width: 100, - height: 200 - } ), - widget = editor.widgets.instances[ 0 ], - markerElement = widget.element, - initialPosition, - currentPosition; - - balloonToolbar.attach( markerElement ); - initialPosition = balloonToolbar.parts.panel.getClientRect(); - - editor.once( 'change', function() { - resume( function() { - currentPosition = balloonToolbar.parts.panel.getClientRect(); - assert.areNotSame( initialPosition.left, currentPosition.left, 'position of toolbar' ); - } ); - } ); - - markerElement.setStyle( 'margin-left', '200px' ); - editor.fire( 'change' ); - - wait(); - } ); - } - }; - - tests = bender.tools.createTestsForEditors( CKEDITOR.tools.objectKeys( bender.editors ), tests ); - ignoreUnsupportedEnvironment( tests ); - bender.test( tests ); -} )(); From 7aca3969cbbffdcbb8b3eb51339da4f56286d045 Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Mon, 18 Dec 2017 12:44:07 +0100 Subject: [PATCH 09/11] Remove unnecessary ignore. --- tests/plugins/balloontoolbar/positioning.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/plugins/balloontoolbar/positioning.js b/tests/plugins/balloontoolbar/positioning.js index 7182731bf96..537d3aef6d9 100644 --- a/tests/plugins/balloontoolbar/positioning.js +++ b/tests/plugins/balloontoolbar/positioning.js @@ -37,10 +37,6 @@ var tests = { setUp: function() { - // In IE8 tests are run in very small window which breaks positioning assertions and tests fails (#1076). - if ( CKEDITOR.env.ie && CKEDITOR.env.version < 9 ) { - assert.ignore(); - } if ( parentFrame ) { parentFrame.style.height = '900px'; } @@ -168,6 +164,7 @@ }; tests = bender.tools.createTestsForEditors( CKEDITOR.tools.objectKeys( bender.editors ), tests ); + // In IE8 tests are run in very small window which breaks positioning assertions and tests fails (#1076). ignoreUnsupportedEnvironment( tests ); bender.test( tests ); } )(); From 64dd244b0bf728b42f9a24f7ba5936a70ccc2a54 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 19 Dec 2017 17:47:52 +0100 Subject: [PATCH 10/11] Removed redundant comment. Tests are not ran on IE8, so there's no need for that. --- tests/plugins/balloontoolbar/positioning.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/plugins/balloontoolbar/positioning.js b/tests/plugins/balloontoolbar/positioning.js index 537d3aef6d9..e94d9413fa6 100644 --- a/tests/plugins/balloontoolbar/positioning.js +++ b/tests/plugins/balloontoolbar/positioning.js @@ -164,7 +164,6 @@ }; tests = bender.tools.createTestsForEditors( CKEDITOR.tools.objectKeys( bender.editors ), tests ); - // In IE8 tests are run in very small window which breaks positioning assertions and tests fails (#1076). ignoreUnsupportedEnvironment( tests ); bender.test( tests ); } )(); From c2893d0698109274e2e3a907a18fc451782a8c4f Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 19 Dec 2017 17:53:42 +0100 Subject: [PATCH 11/11] Changelog entry. --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 67ef6085c5e..dd1fd39e5c7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,6 +5,7 @@ Fixed Issues: +* [#1342](https://github.com/ckeditor/ckeditor-dev/issues/1342): Fixed: [Balloon Toolbar](https://ckeditor.com/cke4/addon/balloontoolbar) should be re-positioned after a [change](https://docs.ckeditor.com/ckeditor4/docs/#!/api/CKEDITOR.editor-event-change) event. * [#1048](https://github.com/ckeditor/ckeditor-dev/issues/1048): Fixed: [Balloon Panel](https://ckeditor.com/cke4/addon/balloonpanel) is not properly positioned when margin added to its non-static parent. * [#889](https://github.com/ckeditor/ckeditor-dev/issues/889): Fixed: Unclear error message for width and height fields in [Image](https://ckeditor.com/cke4/addon/image) and [Enhanced Image](https://ckeditor.com/cke4/addon/image2) plugins. * [#859](https://github.com/ckeditor/ckeditor-dev/issues/859): Fixed: Can't edit link after double click on text in link.