From 5dc21587204935769835c58e30a446d8a9e2eab6 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 12 Feb 2014 12:03:37 +0100 Subject: [PATCH 1/4] Prevent from changing cell properties when the field's value remains unaltered. --- plugins/tabletools/dialogs/tableCell.js | 27 +++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/plugins/tabletools/dialogs/tableCell.js b/plugins/tabletools/dialogs/tableCell.js index b259032fab6..27fcad451e3 100644 --- a/plugins/tabletools/dialogs/tableCell.js +++ b/plugins/tabletools/dialogs/tableCell.js @@ -413,6 +413,33 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { this._.editor.forceNextSelectionCheck(); selection.selectBookmarks( bookmarks ); this._.editor.selectionChange(); + }, + onLoad: function( a ) { + var saved = {}; + + // Prevent from changing cell properties when the field's value + // remains unaltered, i.e. when selected multiple cells and dialog loaded + // only the properties of the first cell (#11439). + this.foreach( function( field ) { + if ( !field.setup || !field.commit ) + return; + + // Save field's value every time after "setup" is called. + field.setup = CKEDITOR.tools.override( field.setup, function( orgSetup ) { + return function() { + orgSetup.apply( this, arguments ); + saved[ field.id ] = field.getValue(); + }; + } ); + + // Compare saved value with actual value. Update cell only if value has changed. + field.commit = CKEDITOR.tools.override( field.commit, function( orgCommit ) { + return function() { + if ( saved[ field.id ] !== field.getValue() ) + orgCommit.apply( this, arguments ); + }; + } ); + } ); } }; } ); From ef38dd06f91bae3b67af71058c10138f5764c157 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 12 Feb 2014 16:52:48 +0100 Subject: [PATCH 2/4] Leave the value of the field empty if it'd differ among selected cells. --- plugins/tabletools/dialogs/tableCell.js | 109 ++++++++++++++++-------- 1 file changed, 72 insertions(+), 37 deletions(-) diff --git a/plugins/tabletools/dialogs/tableCell.js b/plugins/tabletools/dialogs/tableCell.js index 27fcad451e3..97dc31c1f69 100644 --- a/plugins/tabletools/dialogs/tableCell.js +++ b/plugins/tabletools/dialogs/tableCell.js @@ -15,6 +15,41 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { rtl = editor.lang.dir == 'rtl', colorDialog = editor.plugins.colordialog; + // Returns a function, which runs regular "setup" for all selected cells to find out + // whether the initial value of the field would be the same for all cells. If so, + // the value is displayed just as if a regular "setup" was executed. Otherwise, + // i.e. when there are several cells of different value of the property, a field + // gets empty value. + // + // * @param {Function} setup Setup function which returns a value instead of setting it. + // * @returns {Function} A function to be used in dialog definition. + function setupCells( setup ) { + return function( cells ) { + var fieldValue = setup( cells[ 0 ] ); + + // If one of the cells would have a different value of the + // property, set the empty value for a field. + for ( var i = 1; i < cells.length; i++ ) { + if ( setup( cells[ i ] ) !== fieldValue ) { + fieldValue = null; + break; + } + } + + // Setting meaningful or empty value only makes sense + // when setup returns some value. Otherwise, a *default* value + // is used for that field. + if ( typeof fieldValue != 'undefined' ) { + this.setValue( fieldValue ); + + // The only way to have an empty select value in Firefox is + // to set a negative selectedIndex. + if ( CKEDITOR.env.gecko && this.type == 'select' && !fieldValue ) + this.getInputElement().$.selectedIndex = -1; + } + }; + } + return { title: langCell.title, minWidth: CKEDITOR.env.ie && CKEDITOR.env.quirks ? 450 : 410, @@ -54,13 +89,13 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { inputElement.setAttribute( 'aria-labelledby', [ ariaLabelledByAttr, labelElement.$.id ].join( ' ' ) ); }, - setup: function( element ) { + setup: setupCells( function( element ) { var widthAttr = parseInt( element.getAttribute( 'width' ), 10 ), widthStyle = parseInt( element.getStyle( 'width' ), 10 ); - !isNaN( widthAttr ) && this.setValue( widthAttr ); - !isNaN( widthStyle ) && this.setValue( widthStyle ); - }, + return !isNaN( widthStyle ) ? widthStyle : + !isNaN( widthAttr ) ? widthAttr : ''; + } ), commit: function( element ) { var value = parseInt( this.getValue(), 10 ), unit = this.getDialog().getValueOf( 'info', 'widthType' ); @@ -84,11 +119,11 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { [ langTable.widthPx, 'px' ], [ langTable.widthPc, '%' ] ], - setup: function( selectedCell ) { + setup: setupCells( function( selectedCell ) { var widthMatch = widthPattern.exec( selectedCell.getStyle( 'width' ) || selectedCell.getAttribute( 'width' ) ); if ( widthMatch ) - this.setValue( widthMatch[ 2 ] ); - } + return widthMatch[ 2 ]; + } ) } ] }, @@ -114,13 +149,13 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { inputElement.setAttribute( 'aria-labelledby', [ ariaLabelledByAttr, labelElement.$.id ].join( ' ' ) ); }, - setup: function( element ) { + setup: setupCells( function( element ) { var heightAttr = parseInt( element.getAttribute( 'height' ), 10 ), heightStyle = parseInt( element.getStyle( 'height' ), 10 ); - !isNaN( heightAttr ) && this.setValue( heightAttr ); - !isNaN( heightStyle ) && this.setValue( heightStyle ); - }, + return !isNaN( heightStyle ) ? heightStyle : + !isNaN( heightAttr ) ? heightAttr : ''; + } ), commit: function( element ) { var value = parseInt( this.getValue(), 10 ); @@ -149,13 +184,13 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { [ langCell.yes, 'yes' ], [ langCell.no, 'no' ] ], - setup: function( element ) { + setup: setupCells( function( element ) { var wordWrapAttr = element.getAttribute( 'noWrap' ), wordWrapStyle = element.getStyle( 'white-space' ); if ( wordWrapStyle == 'nowrap' || wordWrapAttr ) - this.setValue( 'no' ); - }, + return 'no'; + } ), commit: function( element ) { if ( this.getValue() == 'no' ) element.setStyle( 'white-space', 'nowrap' ); @@ -177,12 +212,12 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { [ langCommon.alignCenter, 'center' ], [ langCommon.alignRight, 'right' ] ], - setup: function( element ) { + setup: setupCells( function( element ) { var alignAttr = element.getAttribute( 'align' ), textAlignStyle = element.getStyle( 'text-align' ); - this.setValue( textAlignStyle || alignAttr || '' ); - }, + return textAlignStyle || alignAttr || ''; + } ), commit: function( selectedCell ) { var value = this.getValue(); @@ -206,7 +241,7 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { [ langCommon.alignBottom, 'bottom' ], [ langCell.alignBaseline, 'baseline' ] ], - setup: function( element ) { + setup: setupCells( function( element ) { var vAlignAttr = element.getAttribute( 'vAlign' ), vAlignStyle = element.getStyle( 'vertical-align' ); @@ -221,8 +256,8 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { vAlignStyle = ''; } - this.setValue( vAlignStyle || vAlignAttr || '' ); - }, + return vAlignStyle || vAlignAttr || ''; + } ), commit: function( element ) { var value = this.getValue(); @@ -250,9 +285,9 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { [ langCell.data, 'td' ], [ langCell.header, 'th' ] ], - setup: function( selectedCell ) { - this.setValue( selectedCell.getName() ); - }, + setup: setupCells( function( selectedCell ) { + return selectedCell.getName(); + } ), commit: function( selectedCell ) { selectedCell.renameNode( this.getValue() ); } @@ -264,11 +299,11 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { label: langCell.rowSpan, 'default': '', validate: validate.integer( langCell.invalidRowSpan ), - setup: function( selectedCell ) { + setup: setupCells( function( selectedCell ) { var attrVal = parseInt( selectedCell.getAttribute( 'rowSpan' ), 10 ); if ( attrVal && attrVal != 1 ) - this.setValue( attrVal ); - }, + return attrVal; + } ), commit: function( selectedCell ) { var value = parseInt( this.getValue(), 10 ); if ( value && value != 1 ) @@ -283,11 +318,11 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { label: langCell.colSpan, 'default': '', validate: validate.integer( langCell.invalidColSpan ), - setup: function( element ) { + setup: setupCells( function( element ) { var attrVal = parseInt( element.getAttribute( 'colSpan' ), 10 ); if ( attrVal && attrVal != 1 ) - this.setValue( attrVal ); - }, + return attrVal; + } ), commit: function( selectedCell ) { var value = parseInt( this.getValue(), 10 ); if ( value && value != 1 ) @@ -307,12 +342,12 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { id: 'bgColor', label: langCell.bgColor, 'default': '', - setup: function( element ) { + setup: setupCells( function( element ) { var bgColorAttr = element.getAttribute( 'bgColor' ), bgColorStyle = element.getStyle( 'background-color' ); - this.setValue( bgColorStyle || bgColorAttr ); - }, + return bgColorStyle || bgColorAttr; + } ), commit: function( selectedCell ) { var value = this.getValue(); @@ -354,12 +389,12 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { id: 'borderColor', label: langCell.borderColor, 'default': '', - setup: function( element ) { + setup: setupCells( function( element ) { var borderColorAttr = element.getAttribute( 'borderColor' ), borderColorStyle = element.getStyle( 'border-color' ); - this.setValue( borderColorStyle || borderColorAttr ); - }, + return borderColorStyle || borderColorAttr; + } ), commit: function( selectedCell ) { var value = this.getValue(); if ( value ) @@ -400,7 +435,7 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { ], onShow: function() { this.cells = CKEDITOR.plugins.tabletools.getSelectedCells( this._.editor.getSelection() ); - this.setupContent( this.cells[ 0 ] ); + this.setupContent( this.cells ); }, onOk: function() { var selection = this._.editor.getSelection(), @@ -414,7 +449,7 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { selection.selectBookmarks( bookmarks ); this._.editor.selectionChange(); }, - onLoad: function( a ) { + onLoad: function() { var saved = {}; // Prevent from changing cell properties when the field's value From b45dd161816c34aab4fd132dde7819acbb00f93f Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 13 Feb 2014 10:16:13 +0100 Subject: [PATCH 3/4] Extracted getCellWidthType function to allow setting the same width for cells of different widthType. --- plugins/tabletools/dialogs/tableCell.js | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/plugins/tabletools/dialogs/tableCell.js b/plugins/tabletools/dialogs/tableCell.js index 97dc31c1f69..8e9e85e6064 100644 --- a/plugins/tabletools/dialogs/tableCell.js +++ b/plugins/tabletools/dialogs/tableCell.js @@ -50,6 +50,18 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { }; } + // Reads the unit of width property of the table cell. + // + // * @param {CKEDITOR.dom.element} cell An element representing table cell. + // * @returns {String} A unit of width: 'px', '%' or undefined if none. + function getCellWidthType( cell ) { + var match = widthPattern.exec( + cell.getStyle( 'width' ) || cell.getAttribute( 'width' ) ); + + if ( match ) + return match[ 2 ]; + } + return { title: langCell.title, minWidth: CKEDITOR.env.ie && CKEDITOR.env.quirks ? 450 : 410, @@ -98,7 +110,12 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { } ), commit: function( element ) { var value = parseInt( this.getValue(), 10 ), - unit = this.getDialog().getValueOf( 'info', 'widthType' ); + + // There might be no widthType value, i.e. when multiple cells are + // selected but some of them have width expressed in pixels and some + // of them in percent. Try to re-read the unit from the cell in such + // case (#11439). + unit = this.getDialog().getValueOf( 'info', 'widthType' ) || getCellWidthType( element ); if ( !isNaN( value ) ) element.setStyle( 'width', value + unit ); @@ -119,11 +136,7 @@ CKEDITOR.dialog.add( 'cellProperties', function( editor ) { [ langTable.widthPx, 'px' ], [ langTable.widthPc, '%' ] ], - setup: setupCells( function( selectedCell ) { - var widthMatch = widthPattern.exec( selectedCell.getStyle( 'width' ) || selectedCell.getAttribute( 'width' ) ); - if ( widthMatch ) - return widthMatch[ 2 ]; - } ) + setup: setupCells( getCellWidthType ) } ] }, From ccfac63cff8a9f684a55227f2aed95606086d586 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 19 Feb 2014 11:20:02 +0100 Subject: [PATCH 4/4] Changelog entry. --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index cc68e9fcad3..cdbd1434841 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -26,6 +26,7 @@ Fixed Issues: * [#11542](http://dev.ckeditor.com/ticket/11542): [IE11] Fixed: Blurry toolbar icons when right-to-left UI language is set. * [#11504](http://dev.ckeditor.com/ticket/11504): Fixed: When `config.fullPage` is set `true`, entities are not encoded in editor's output. * [#11004](http://dev.ckeditor.com/ticket/11004): Integrated [Enhanced Image](http://ckeditor.com/addon/image2) dialog with [Advanced Content Filter](http://docs.ckeditor.com/#!/guide/dev_advanced_content_filter). +* [#11439](http://dev.ckeditor.com/ticket/11439): Fixed: Properties get cloned in Cell Properties dialog if multiple cells are selected. ## CKEditor 4.3.2