Only strip style attributes on rich paste from VE

Stripping all HTML atributes (to avoid CE-added styles such as
'font-size: 1em;') also strips data-parsoid which can cause
round trip errors. As an improvement only strip the style
attribute.

Bug: 58136
Change-Id: I34386bd847d1cf0583317a8b07916e43ff7af029

modules/ve/ce/ve.ce.Surface.js

@@ -990,7 +990,7 @@ ve.ce.Surface.prototype.afterPaste = function () {
 			// ...except not quite - contentEditable can't be trusted not
 			// to add styles, so for now remove them
 			// TODO: store original styles in data
-			data.sanitize( { 'removeHtmlAttributes': true } );
+			data.sanitize( { 'removeStyles': true } );
 		}
 		data.remapInternalListKeys( this.model.getDocument().getInternalList() );
 

modules/ve/dm/lineardata/ve.dm.ElementLinearData.js

@@ -816,7 +816,8 @@ ve.dm.ElementLinearData.prototype.remapInternalListKeys = function ( internalLis
  *
  * @param {Object} rules Sanitization rules
  * @param {string[]} [rules.blacklist] Blacklist of model types which aren't allowed
- * @param {boolean} [rules.removeHtmlAttributes] Remove all left over HTML attributes and any empty spans it creates
+ * @param {boolean} [rules.removeHtmlAttributes] Remove all left over HTML attributes
+ * @param {boolean} [rules.removeStyles] Remove HTML style attributes
  * @param {boolean} [plainText=false] Remove all formatting for plain text paste
  * @param {boolean} [keepEmptyContentBranches=false] Preserve empty content branch nodes
  */
@@ -824,6 +825,25 @@ ve.dm.ElementLinearData.prototype.sanitize = function ( rules, plainText, keepEm
 	var i, len, annotations, emptySet, setToRemove, type,
 		allAnnotations = this.getAnnotationsFromRange( new ve.Range( 0, this.getLength() ), true );
 
+	function removeHtmlAttribute( element, attribute ) {
+		var i;
+		if ( element.htmlAttributes ) {
+			for ( i = 0; i < element.htmlAttributes.length; i++ ) {
+				delete element.htmlAttributes[i].values[attribute];
+				if ( ve.isEmptyObject( element.htmlAttributes[i].values ) ) {
+					delete element.htmlAttributes[i].values;
+				}
+				if ( ve.isEmptyObject( element.htmlAttributes[i] ) ) {
+					element.htmlAttributes.splice( i, 1 );
+					i--;
+				}
+			}
+			if ( !element.htmlAttributes.length ) {
+				delete element.htmlAttributes;
+			}
+		}
+	}
+
 	if ( plainText ) {
 		emptySet = new ve.dm.AnnotationSet( this.getStore() );
 	} else {
@@ -833,6 +853,12 @@ ve.dm.ElementLinearData.prototype.sanitize = function ( rules, plainText, keepEm
 				delete allAnnotations.get( i ).element.htmlAttributes;
 			}
 		}
+		if ( rules.removeStyles ) {
+			for ( i = 0, len = allAnnotations.getLength(); i < len; i++ ) {
+				// Remove inline style attributes from annotations
+				removeHtmlAttribute( allAnnotations.get( i ).element, 'style' );
+			}
+		}
 
 		// Create annotation set to remove from blacklist
 		setToRemove = allAnnotations.filter( function ( annotation ) {
@@ -878,15 +904,21 @@ ve.dm.ElementLinearData.prototype.sanitize = function ( rules, plainText, keepEm
 		if ( !annotations.isEmpty() ) {
 			if ( plainText ) {
 				this.setAnnotationsAtOffset( i, emptySet );
-			} else {
+			} else if ( setToRemove.getLength() ) {
 				// Remove blacklisted annotations
 				annotations.removeSet( setToRemove );
 				this.setAnnotationsAtOffset( i, annotations );
 			}
 		}
-		if ( rules.removeHtmlAttributes && this.isOpenElementData( i ) ) {
-			// Remove HTML attributes from nodes
-			delete this.getData( i ).htmlAttributes;
+		if ( this.isOpenElementData( i ) ) {
+			if ( rules.removeHtmlAttributes ) {
+				// Remove HTML attributes from nodes
+				delete this.getData( i ).htmlAttributes;
+			}
+			if ( rules.removeStyles ) {
+				// Remove inline style attributes from nodes
+				removeHtmlAttribute( this.getData( i ), 'style' );
+			}
 		}
 	}
 };

modules/ve/init/ve.init.Target.js

@@ -73,6 +73,5 @@ ve.init.Target.static.pasteRules = {
 		'textStyle/span',
 		// Nodes
 		'alienInline', 'alienBlock'
-	],
-	'removeHtmlAttributes': false
+	]
 };

modules/ve/test/dm/lineardata/ve.dm.ElementLinearData.test.js

@@ -1370,9 +1370,16 @@ QUnit.test( 'sanitize', function ( assert ) {
 	var i, model, data,
 		count = 0,
 		bold = new ve.dm.TextStyleBoldAnnotation( { 'type': 'textStyle/bold', 'attributes': { 'nodeName': 'b' } } ),
+		boldWithClass = new ve.dm.TextStyleBoldAnnotation( {
+			'type': 'textStyle/bold',
+			'attributes': { 'nodeName': 'b' },
+			'htmlAttributes': [ {
+				'values': { 'class': 'bar' }
+			} ]
+		} ),
 		cases = [
 			{
-				'html': '<p style="text-shadow: 0 0 1px #000;">F<b>o</b>o</p>',
+				'html': '<p style="text-shadow: 0 0 1px #000;">F<b style="color:blue;">o</b>o</p>',
 				'data': [
 					{ 'type': 'paragraph' },
 					'F', ['o', [0]], 'o',
@@ -1421,6 +1428,37 @@ QUnit.test( 'sanitize', function ( assert ) {
 					{ 'type': '/internalList' }
 				],
 				'msg': 'Empty content nodes are stripped'
+			},
+			{
+				'html': '<p style="font-size: 2em;"><b style="color:red;">Foo</b></p>',
+				'data': [
+					{ 'type': 'paragraph' },
+					['F',[0]], ['o',[0]], ['o',[0]],
+					{ 'type': '/paragraph' },
+					{ 'type': 'internalList' },
+					{ 'type': '/internalList' }
+				],
+				'store': [ bold ],
+				'rules': { 'removeStyles': true },
+				'msg': 'Style attribute removed and htmlAttributes unset'
+			},
+			{
+				'html': '<p style="font-size: 2em;" class="foo"><b style="color:red;" class="bar">Foo</b></p>',
+				'data': [
+					{
+						'type': 'paragraph',
+						'htmlAttributes': [ {
+							'values': { 'class': 'foo' }
+						} ]
+					},
+					['F',[0]], ['o',[0]], ['o',[0]],
+					{ 'type': '/paragraph' },
+					{ 'type': 'internalList' },
+					{ 'type': '/internalList' }
+				],
+				'store': [ boldWithClass ],
+				'rules': { 'removeStyles': true },
+				'msg': 'Style attribute removed and other attributes preserved'
 			}
 		];
 

modules/ve/ui/ve.ui.Surface.js

@@ -299,9 +299,8 @@ ve.ui.Surface.prototype.getPasteRules = function () {
 /**
  * Set sanitization rules for rich paste
  *
+ * @see ve.dm.ElementLinearData#sanitize
  * @param {Object} pasteRules Paste rules
- * @param {string[]} [pasteRules.blacklist] Blacklist of model types which aren't allowed
- * @param {boolean} [pasteRules.removeHtmlAttributes] Remove all left over HTML attributes
  */
 ve.ui.Surface.prototype.setPasteRules = function ( pasteRules ) {
 	this.pasteRules = pasteRules;