Merge pull request #580 from ckeditor/t/565

Auto adding `#` to hex color on colordialog close.

CHANGES.md

@@ -3,6 +3,10 @@
 
 ## CKEditor 4.8
 
+New Features:
+
+* [#607](https://github.com/ckeditor/ckeditor-dev/issues/607): Manually inserted hex color is prefixed with hash tag if needed. It ensures a valid hex color is used when setting table cell border or background color via [Color Dialog](http://ckeditor.com/addon/colordialog) window.
+
 ## CKEditor 4.7.1
 
 New Features:

plugins/colordialog/dialogs/colordialog.js

@@ -22,6 +22,14 @@ CKEDITOR.dialog.add( 'colordialog', function( editor ) {
 		html: ' '
 	};
 
+	var numbering = function( id ) {
+			return CKEDITOR.tools.getNextId() + '_' + id;
+		},
+		hicolorId = numbering( 'hicolor' ),
+		hicolorTextId = numbering( 'hicolortext' ),
+		selHiColorId = numbering( 'selhicolor' ),
+		table;
+
 	function clearSelected() {
 		$doc.getById( selHiColorId ).removeStyle( 'background-color' );
 		dialog.getContentElement( 'picker', 'selectedColor' ).setValue( '' );
@@ -85,8 +93,10 @@ CKEDITOR.dialog.add( 'colordialog', function( editor ) {
 	}
 
 	function clearHighlight() {
-		focused.removeClass( focusedColorLightCls );
-		focused.removeClass( focusedColorDarkCls );
+		if ( focused ) {
+			focused.removeClass( focusedColorLightCls );
+			focused.removeClass( focusedColorDarkCls );
+		}
 		setHighlight( false );
 		focused = null;
 	}
@@ -257,14 +267,6 @@ CKEDITOR.dialog.add( 'colordialog', function( editor ) {
 		appendColorCell( oRow.$, '#ffffff' );
 	}
 
-	var numbering = function( id ) {
-			return CKEDITOR.tools.getNextId() + '_' + id;
-		},
-		hicolorId = numbering( 'hicolor' ),
-		hicolorTextId = numbering( 'hicolortext' ),
-		selHiColorId = numbering( 'selhicolor' ),
-		table;
-
 	createColorTable();
 
 	// Load CSS.

plugins/colordialog/plugin.js

@@ -25,16 +25,24 @@ CKEDITOR.plugins.colordialog = {
 		 * @member CKEDITOR.editor
 		 */
 		editor.getColorFromDialog = function( callback, scope ) {
-			var onClose = function( evt ) {
+			var onClose,
+				releaseHandlers,
+				bindToDialog;
+
+			onClose = function( evt ) {
 				releaseHandlers( this );
 				var color = evt.name == 'ok' ? this.getValueOf( 'picker', 'selectedColor' ) : null;
+				// Adding `#` character to hexadecimal 3 or 6 digit numbers to have proper color value (#565).
+				if ( /^[0-9a-f]{3}([0-9a-f]{3})?$/i.test( color ) ) {
+					color = '#' + color;
+				}
 				callback.call( scope, color );
 			};
-			var releaseHandlers = function( dialog ) {
+			releaseHandlers = function( dialog ) {
 				dialog.removeListener( 'ok', onClose );
 				dialog.removeListener( 'cancel', onClose );
 			};
-			var bindToDialog = function( dialog ) {
+			bindToDialog = function( dialog ) {
 				dialog.on( 'ok', onClose );
 				dialog.on( 'cancel', onClose );
 			};

tests/plugins/colordialog/colordialog.js

@@ -0,0 +1,56 @@
+/* bender-tags: editor */
+/* bender-ckeditor-plugins: colordialog,wysiwygarea */
+
+( function() {
+	'use strict';
+
+	bender.editor = true;
+
+	bender.test( {
+		assertColor: function( inputColor, outputColor ) {
+			var editor = this.editor;
+
+			editor.once( 'dialogShow', function( evt ) {
+				var dialog = evt.data;
+				dialog.setValueOf( 'picker', 'selectedColor', inputColor );
+				dialog.getButton( 'ok' ).click();
+
+			} );
+
+			editor.getColorFromDialog( function( color ) {
+				resume( function() {
+					assert.areSame( outputColor, color );
+				} );
+			} );
+			wait();
+		},
+
+		'test colordialog add hash to color\'s values with 6 hexadecimal digits': function() {
+			this.assertColor( '123456', '#123456' );
+		},
+
+		'test colordialog add hash to color\'s values with 3 hexadecimal digits': function() {
+			this.assertColor( 'FDE', '#FDE' );
+		},
+
+		'test colordialog does not add hash to color value with 1 digit (incorrect css color value)': function() {
+			// IE8 doesn't set incorrect values.
+			if ( CKEDITOR.env.ie && CKEDITOR.env.version < 9 ) {
+				assert.ignore();
+			}
+			this.assertColor( '1', '1' );
+		},
+
+		'test colordialog does not add hash to color name': function() {
+			this.assertColor( 'red', 'red' );
+		},
+
+		'test colordialog does not add hash to rgb color value': function() {
+			this.assertColor( 'rgb(10, 20, 30)', 'rgb(10, 20, 30)' );
+		},
+
+		'test colordialog does not add hash to empty value ': function() {
+			this.assertColor( '', '' );
+		}
+	} );
+} )();

tests/plugins/colordialog/manual/addhash.html

@@ -0,0 +1,15 @@
+<textarea id="editor1">
+	<table border="1" cellspacing="1" cellpadding="1">
+		<tr>
+			<td>Cell 1</td><td>Cell 2</td>
+		</tr>
+	</table>
+</textarea>
+
+<script>
+	if ( bender.tools.env.mobile ) {
+		bender.ignore();
+	} else {
+		CKEDITOR.replace( 'editor1' );
+	}
+</script>

tests/plugins/colordialog/manual/addhash.md

@@ -0,0 +1,14 @@
+@bender-tags: tc, feature, colordialog, 565, 4.8.0
+@bender-ui: collapsed
+@bender-ckeditor-plugins: table,tabletools,colordialog,toolbar,wysiwygarea,sourcearea,contextmenu
+
+1. Right click into table cell.
+1. Select `Cell -> Cell Properties`.
+1. Press `Choose` button next to _Background Color_.
+	* Color dialog should appear.
+1. Type color value in input field (just above `Clear` button) in hexadecimal 6-digit format **without** leading `#`. E.g. (`ABC123`).
+1. Confirm it.
+	* **Expected**: Inserted color should contain additional `#` at the beginning.
+1. Repeat above steps for color with hexadecimal 3-digits format. E.g. (`FFF`).
+
+**Note:** You can also check the behaviour of color dialog by setting values in different formats (rgb, text) or invalid hex strings. For such colors `#` should not be added.