Merge branch 't/12140'

CHANGES.md

@@ -5,6 +5,7 @@ CKEditor 4 Changelog
 
 Fixed Issues:
 
+* [#12140](http://dev.ckeditor.com/ticket/12140): Double-clicking linked widgets will no longer open two dialogs.
 * [#12110](http://dev.ckeditor.com/ticket/12110): Fixed: Editor crash after deleting a table. Thanks to [Alin Purcaru](https://github.com/mesmerizero)!
 * [#11897](http://dev.ckeditor.com/ticket/11897): Fixed: Enter key in an empty formatted list item, creates a new line instead of breaking the list. Thanks to [noam-si](https://github.com/noam-si)!
 

plugins/image2/plugin.js

@@ -1341,21 +1341,6 @@
 		if ( !editor.plugins.link )
 			return;
 
-		// Generally speaking, link dialog does not open if double-clicked
-		// linked widget because widget internals are read-only:
-		//
-		//		<wrapper><a><img/></a></wrapper> // <a> is not editable
-		//
-		// However, in the following case:
-		//
-		//		<a>x<wrapper><img/></wrapper>x</a> // <a> is editable
-		//
-		// it would open along with Image2 dialog, if not blocked.
-		editor.on( 'doubleclick', function( evt ) {
-			if ( evt.data.dialog && evt.data.dialog == 'link' && getFocusedWidget( editor ) )
-				evt.cancel();
-		} );
-
 		CKEDITOR.on( 'dialogDefinition', function( evt ) {
 			var dialog = evt.data;
 

plugins/widget/plugin.js

@@ -1034,14 +1034,16 @@
 		 *
 		 * The dialog window name is obtained from the event's data `dialog` property or
 		 * from {@link CKEDITOR.plugins.widget.definition#dialog}.
+		 *
+		 * @returns {Boolean} Returns `true` if dialog was opened.
 		 */
 		edit: function() {
 			var evtData = { dialog: this.dialog },
 				that = this;
 
 			// Edit event was blocked, but there's no dialog to be automatically opened.
 			if ( this.fire( 'edit', evtData ) === false || !evtData.dialog )
-				return;
+				return false;
 
 			this.editor.openDialog( evtData.dialog, function( dialog ) {
 				var showListener,
@@ -1083,6 +1085,8 @@
 					okListener.removeListener();
 				} );
 			} );
+
+			return true;
 		},
 
 		/**
@@ -1452,6 +1456,19 @@
 	/**
 	 * An event fired when a widget is double clicked.
 	 *
+	 * **Note:** if a default editing action is executed on double click (i.e. widget has a
+	 * {@link CKEDITOR.plugins.widget.definition#dialog dialog} defined and the {@link #event-doubleclick} event was not
+	 * cancelled) this event will be automatically cancelled, so listener added with the default priority (10)
+	 * will not be executed. Use listener with low priority (e.g. 5) to be sure that it will be executed.
+	 *
+	 *		widget.on( 'doubleclick', function( evt ) {
+	 *			console.log( 'widget#doubleclick' );
+	 *		}, null, null, 5 );
+	 *
+	 * If your widget handles double click in a special way (so default editing action is not executed)
+	 * make sure to cancel this event, because otherwise it will be propagated to {@link CKEDITOR.editor#doubleclick}
+	 * and other feature may step in (e.g. link dialog may be opened if your widget was inside a link).
+	 *
 	 * @event doubleclick
 	 * @param data
 	 * @param {CKEDITOR.dom.element} data.element The double clicked element.
@@ -3115,7 +3132,11 @@
 		// to overwrite this callback.
 
 		widget.on( 'doubleclick', function( evt ) {
-			widget.edit();
+			if ( widget.edit() ) {
+				// We have to cancel event if edit method opens a dialog, otherwise
+				// link plugin may open extra dialog (#12140).
+				evt.cancel();
+			}
 		} );
 
 		if ( widgetDef.data )

tests/plugins/widget/editing.js

@@ -69,17 +69,19 @@
 
 			this.editorBot.setData( '<p data-widget="test1" id="x">foo</p>', function() {
 				var widget = getWidgetById( editor, 'x' ),
-					openedDialog;
+					openedDialog,
+					retVal;
 
 				var revert = replaceMethod( editor, 'openDialog', function( dialogName ) {
 					openedDialog = dialogName;
 				} );
 
-				widget.edit();
+				retVal = widget.edit();
 
 				revert();
 
 				assert.areSame( 'widgettest1', openedDialog );
+				assert.isTrue( retVal, 'widget.edit() return value' );
 			} );
 		},
 
@@ -88,17 +90,19 @@
 
 			this.editorBot.setData( '<p data-widget="test2" id="x">foo</p>', function() {
 				var widget = getWidgetById( editor, 'x' ),
-					openedDialog = null;
+					openedDialog = null,
+					retVal;
 
 				var revert = replaceMethod( editor, 'openDialog', function( dialogName ) {
 					openedDialog = dialogName;
 				} );
 
-				widget.edit();
+				retVal = widget.edit();
 
 				revert();
 
 				assert.isNull( openedDialog );
+				assert.isFalse( retVal, 'widget.edit() return value' );
 			} );
 		},
 
@@ -148,6 +152,34 @@
 			} );
 		},
 
+		'test cancelling doubleclick when widget.edit() returns true': function() {
+			var editor = this.editor;
+
+			this.editorBot.setData( '<p data-widget="test1" id="x">foo</p>', function() {
+				var widget = getWidgetById( editor, 'x' ),
+					widgetElement = editor.document.getById( 'x' ),
+					retVal;
+
+				// Doubleclick should be canceled if widget.edit() returns true.
+				widget.edit = function() {
+					return true;
+				};
+
+				retVal = editor.fire( 'doubleclick', { element: widgetElement } );
+
+				assert.isFalse( retVal, 'editor#doubleclick should be canceled' );
+
+				// Now opposite situation.
+				widget.edit = function() {
+					return false;
+				};
+
+				retVal = editor.fire( 'doubleclick', { element: widgetElement } );
+
+				assert.isTrue( retVal !== false, 'editor#doubleclick should not be canceled' );
+			} );
+		},
+
 		'test setting up data in dialog': function() {
 			var editor = this.editor;