Merge pull request #1826 from ckeditor/i/6091

Other: `StylesProcessor` rules will not be stored in a singleton, which made them shared between editor instances. In order to allow binding a styles processor instance to a specific view document, we had to replace a dynamic `#document` property in view nodes with a static one, set upon node creation. Closes ckeditor/ckeditor5#6091.

MAJOR BREAKING CHANGES: `EditingController` requires an instance of StylesProcessor in its constructor.

MAJOR BREAKING CHANGES: `DataController` requires an instance of StylesProcessor in its constructor.

MAJOR BREAKING CHANGES: `DomConverter`, `HtmlDataProcessor` and `XmlDataProcessor` require an instance of the view document in their constructors.

MAJOR BREAKING CHANGES: The `View` class requires an instance of `StylesProcessor` as its first argument.

MAJOR BREAKING CHANGES: The `createViewElementFromHighlightDescriptor()` function that is exported by `src/conversion/downcasthelpers.js` file requires an instance of the view document as its first argument.

MAJOR BREAKING CHANGES: Method `view.Document#addStyleProcessorRules()` has been moved to DataController class.

MINOR BREAKING CHANGES: `DataController` does not accept the data processor instance any more.

MAJOR BREAKING CHANGES: The `#document` getter was removed from model nodes. Only the root element holds the reference to the model document. For attached nodes, use `node.root.document` to access it.

Author: Reinmar

src/controller/datacontroller.js

@@ -47,10 +47,9 @@ export default class DataController {
 	 * Creates a data controller instance.
 	 *
 	 * @param {module:engine/model/model~Model} model Data model.
-	 * @param {module:engine/dataprocessor/dataprocessor~DataProcessor} [dataProcessor] Data processor that should be used
-	 * by the controller.
+	 * @param {module:engine/view/stylesmap~StylesProcessor} stylesProcessor The styles processor instance.
 	 */
-	constructor( model, dataProcessor ) {
+	constructor( model, stylesProcessor ) {
 		/**
 		 * Data model.
 		 *
@@ -60,12 +59,19 @@ export default class DataController {
 		this.model = model;
 
 		/**
-		 * Data processor used during the conversion.
+		 * Styles processor used during the conversion.
 		 *
 		 * @readonly
-		 * @member {module:engine/dataprocessor/dataprocessor~DataProcessor}
+		 * @member {module:engine/view/stylesmap~StylesProcessor}
 		 */
-		this.processor = dataProcessor;
+		this.stylesProcessor = stylesProcessor;
+
+		/**
+		 * Data processor used during the conversion.
+		 *
+		 * @member {module:engine/dataprocessor/dataprocessor~DataProcessor} #processor
+		 */
+		this.processor;
 
 		/**
 		 * Mapper used for the conversion. It has no permanent bindings, because they are created when getting data and
@@ -187,12 +193,13 @@ export default class DataController {
 
 		// First, convert elements.
 		const modelRange = ModelRange._createIn( modelElementOrFragment );
+		const viewDocument = new ViewDocument( this.stylesProcessor );
 
-		const viewDocumentFragment = new ViewDocumentFragment();
+		const viewDocumentFragment = new ViewDocumentFragment( viewDocument );
 
 		// Create separate ViewDowncastWriter just for data conversion purposes.
 		// We have no view controller and rendering do DOM in DataController so view.change() block is not used here.
-		const viewWriter = new ViewDowncastWriter( new ViewDocument() );
+		const viewWriter = new ViewDowncastWriter( viewDocument );
 		this.mapper.bindElements( modelElementOrFragment, viewDocumentFragment );
 
 		this.downcastDispatcher.convertInsert( modelRange, viewWriter );
@@ -371,6 +378,22 @@ export default class DataController {
 		} );
 	}
 
+	/**
+	 * Adds a style processor normalization rules.
+	 *
+	 * You can implement your own rules as well as use one of the available processor rules:
+	 *
+	 * * background: {@link module:engine/view/styles/background~addBackgroundRules}
+	 * * border: {@link module:engine/view/styles/border~addBorderRules}
+	 * * margin: {@link module:engine/view/styles/margin~addMarginRules}
+	 * * padding: {@link module:engine/view/styles/padding~addPaddingRules}
+	 *
+	 * @param {Function} callback
+	 */
+	addStyleProcessorRules( callback ) {
+		callback( this.stylesProcessor );
+	}
+
 	/**
 	 * Removes all event listeners set by the DataController.
 	 */

src/controller/editingcontroller.js

@@ -31,8 +31,9 @@ export default class EditingController {
 	 * Creates an editing controller instance.
 	 *
 	 * @param {module:engine/model/model~Model} model Editing model.
+	 * @param {module:engine/view/stylesmap~StylesProcessor} stylesProcessor The styles processor instance.
 	 */
-	constructor( model ) {
+	constructor( model, stylesProcessor ) {
 		/**
 		 * Editor model.
 		 *
@@ -47,7 +48,7 @@ export default class EditingController {
 		 * @readonly
 		 * @member {module:engine/view/view~View}
 		 */
-		this.view = new View();
+		this.view = new View( stylesProcessor );
 
 		/**
 		 * Mapper which describes the model-view binding.
@@ -115,10 +116,9 @@ export default class EditingController {
 				return null;
 			}
 
-			const viewRoot = new RootEditableElement( root.name );
+			const viewRoot = new RootEditableElement( this.view.document, root.name );
 
 			viewRoot.rootName = root.rootName;
-			viewRoot._document = this.view.document;
 			this.mapper.bindElements( root, viewRoot );
 
 			return viewRoot;

src/conversion/downcasthelpers.js

@@ -416,11 +416,12 @@ export function remove() {
  * provided by the {@link module:engine/conversion/downcasthelpers~HighlightDescriptor highlight descriptor} object. If a priority
  * is not provided in the descriptor, the default priority will be used.
  *
+ * @param {module:engine/view/downcastwriter~DowncastWriter} writer
  * @param {module:engine/conversion/downcasthelpers~HighlightDescriptor} descriptor
  * @returns {module:engine/view/attributeelement~AttributeElement}
  */
-export function createViewElementFromHighlightDescriptor( descriptor ) {
-	const viewElement = new ViewAttributeElement( 'span', descriptor.attributes );
+export function createViewElementFromHighlightDescriptor( writer, descriptor ) {
+	const viewElement = writer.createAttributeElement( 'span', descriptor.attributes );
 
 	if ( descriptor.classes ) {
 		viewElement._addClass( descriptor.classes );
@@ -543,7 +544,7 @@ export function clearAttributes() {
 			// Not collapsed selection should not have artifacts.
 			if ( range.isCollapsed ) {
 				// Position might be in the node removed by the view writer.
-				if ( range.end.parent.document ) {
+				if ( range.end.parent.isAttached() ) {
 					conversionApi.writer.mergeAttributes( range.start );
 				}
 			}
@@ -919,8 +920,8 @@ function highlightText( highlightDescriptor ) {
 			return;
 		}
 
-		const viewElement = createViewElementFromHighlightDescriptor( descriptor );
 		const viewWriter = conversionApi.writer;
+		const viewElement = createViewElementFromHighlightDescriptor( viewWriter, descriptor );
 		const viewSelection = viewWriter.document.selection;
 
 		if ( data.item instanceof ModelSelection || data.item instanceof DocumentSelection ) {
@@ -1034,7 +1035,7 @@ function removeHighlight( highlightDescriptor ) {
 		}
 
 		// View element that will be used to unwrap `AttributeElement`s.
-		const viewHighlightElement = createViewElementFromHighlightDescriptor( descriptor );
+		const viewHighlightElement = createViewElementFromHighlightDescriptor( conversionApi.writer, descriptor );
 
 		// Get all elements bound with given marker name.
 		const elements = conversionApi.mapper.markerNameToElements( data.markerName );

src/conversion/viewconsumable.js

@@ -9,7 +9,6 @@
 
 import { isArray } from 'lodash-es';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
-import StylesMap from '../view/stylesmap';
 
 /**
  * Class used for handling consumption of view {@link module:engine/view/element~Element elements},
@@ -89,7 +88,7 @@ export default class ViewConsumable {
 
 		// For elements create new ViewElementConsumables or update already existing one.
 		if ( !this._consumables.has( element ) ) {
-			elementConsumables = new ViewElementConsumables();
+			elementConsumables = new ViewElementConsumables( element );
 			this._consumables.set( element, elementConsumables );
 		} else {
 			elementConsumables = this._consumables.get( element );
@@ -239,6 +238,7 @@ export default class ViewConsumable {
 	 */
 	static consumablesFromElement( element ) {
 		const consumables = {
+			element,
 			name: true,
 			attributes: [],
 			classes: [],
@@ -284,7 +284,7 @@ export default class ViewConsumable {
 	 */
 	static createFrom( from, instance ) {
 		if ( !instance ) {
-			instance = new ViewConsumable();
+			instance = new ViewConsumable( from );
 		}
 
 		if ( from.is( 'text' ) ) {
@@ -319,8 +319,17 @@ export default class ViewConsumable {
 class ViewElementConsumables {
 	/**
 	 * Creates ViewElementConsumables instance.
+	 *
+	 * @param {module:engine/view/node~Node|module:engine/view/documentfragment~DocumentFragment} from View node or document fragment
+	 * from which `ViewElementConsumables` is being created.
 	 */
-	constructor() {
+	constructor( from ) {
+		/**
+		 * @readonly
+		 * @member {module:engine/view/node~Node|module:engine/view/documentfragment~DocumentFragment}
+		 */
+		this.element = from;
+
 		/**
 		 * Flag indicating if name of the element can be consumed.
 		 *
@@ -510,7 +519,7 @@ class ViewElementConsumables {
 			consumables.set( name, true );
 
 			if ( type === 'styles' ) {
-				for ( const alsoName of StylesMap.getRelatedStyles( name ) ) {
+				for ( const alsoName of this.element.document.stylesProcessor.getRelatedStyles( name ) ) {
 					consumables.set( alsoName, true );
 				}
 			}
@@ -577,7 +586,7 @@ class ViewElementConsumables {
 				consumables.set( name, false );
 
 				if ( type == 'styles' ) {
-					for ( const toConsume of StylesMap.getRelatedStyles( name ) ) {
+					for ( const toConsume of this.element.document.stylesProcessor.getRelatedStyles( name ) ) {
 						consumables.set( toConsume, false );
 					}
 				}

src/dataprocessor/htmldataprocessor.js

@@ -21,8 +21,10 @@ import DomConverter from '../view/domconverter';
 export default class HtmlDataProcessor {
 	/**
 	 * Creates a new instance of the HTML data processor class.
+	 *
+	 * @param {module:engine/view/document~Document} document The view document instance.
 	 */
-	constructor() {
+	constructor( document ) {
 		/**
 		 * A DOM parser instance used to parse an HTML string to an HTML document.
 		 *
@@ -37,7 +39,7 @@ export default class HtmlDataProcessor {
 		 * @private
 		 * @member {module:engine/view/domconverter~DomConverter}
 		 */
-		this._domConverter = new DomConverter( { blockFillerMode: 'nbsp' } );
+		this._domConverter = new DomConverter( document, { blockFillerMode: 'nbsp' } );
 
 		/**
 		 * A basic HTML writer instance used to convert DOM elements to an HTML string.

src/dataprocessor/xmldataprocessor.js

@@ -24,10 +24,11 @@ export default class XmlDataProcessor {
 	/**
 	 * Creates a new instance of the XML data processor class.
 	 *
+	 * @param {module:engine/view/document~Document} document The view document instance.
 	 * @param {Object} options Configuration options.
 	 * @param {Array<String>} [options.namespaces=[]] A list of namespaces allowed to use in the XML input.
 	 */
-	constructor( options = {} ) {
+	constructor( document, options = {} ) {
 		/**
 		 * A list of namespaces allowed to use in the XML input.
 		 *
@@ -53,7 +54,7 @@ export default class XmlDataProcessor {
 		 * @private
 		 * @member {module:engine/view/domconverter~DomConverter}
 		 */
-		this._domConverter = new DomConverter( { blockFillerMode: 'nbsp' } );
+		this._domConverter = new DomConverter( document, { blockFillerMode: 'nbsp' } );
 
 		/**
 		 * A basic HTML writer instance used to convert DOM elements to an XML string.

src/dev-utils/model.js

@@ -39,6 +39,7 @@ import {
 
 import { isPlainObject } from 'lodash-es';
 import toMap from '@ckeditor/ckeditor5-utils/src/tomap';
+import { StylesProcessor } from '../view/stylesmap';
 
 /**
  * Writes the content of a model {@link module:engine/model/document~Document document} to an HTML-like string.
@@ -210,12 +211,12 @@ export function stringify( node, selectionOrPositionOrRange = null, markers = nu
 
 	// Set up conversion.
 	// Create a temporary view controller.
-	const view = new View();
+	const stylesProcessor = new StylesProcessor();
+	const view = new View( stylesProcessor );
 	const viewDocument = view.document;
-	const viewRoot = new ViewRootEditableElement( 'div' );
+	const viewRoot = new ViewRootEditableElement( viewDocument, 'div' );
 
 	// Create a temporary root element in view document.
-	viewRoot._document = view.document;
 	viewRoot.rootName = 'main';
 	viewDocument.roots.add( viewRoot );
 
@@ -242,7 +243,7 @@ export function stringify( node, selectionOrPositionOrRange = null, markers = nu
 		// Stringify object types values for properly display as an output string.
 		const attributes = convertAttributes( modelItem.getAttributes(), stringifyAttributeValue );
 
-		return new ViewContainerElement( modelItem.name, attributes );
+		return new ViewContainerElement( viewDocument, modelItem.name, attributes );
 	} ) );
 
 	downcastDispatcher.on( 'selection', convertRangeSelection() );

src/dev-utils/view.js

@@ -14,6 +14,7 @@
  */
 
 import View from '../view/view';
+import ViewDocument from '../view/document';
 import ViewDocumentFragment from '../view/documentfragment';
 import XmlDataProcessor from '../dataprocessor/xmldataprocessor';
 import ViewElement from '../view/element';
@@ -24,6 +25,7 @@ import AttributeElement from '../view/attributeelement';
 import ContainerElement from '../view/containerelement';
 import EmptyElement from '../view/emptyelement';
 import UIElement from '../view/uielement';
+import { StylesProcessor } from '../view/stylesmap';
 
 const ELEMENT_RANGE_START_TOKEN = '[';
 const ELEMENT_RANGE_END_TOKEN = ']';
@@ -321,16 +323,19 @@ export function stringify( node, selectionOrPositionOrRange = null, options = {}
  * this node will be used as the root for all parsed nodes.
  * @param {Boolean} [options.sameSelectionCharacters=false] When set to `false`, the selection inside the text should be marked using
  * `{` and `}` and the selection outside the ext using `[` and `]`. When set to `true`, both should be marked with `[` and `]` only.
+ * @param {module:engine/view/stylesmap~StylesProcessor} [options.stylesProcessor] Styles processor.
  * @returns {module:engine/view/text~Text|module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment|Object}
  * Returns the parsed view node or an object with two fields: `view` and `selection` when selection ranges were included in the data
  * to parse.
  */
 export function parse( data, options = {} ) {
+	const viewDocument = new ViewDocument( new StylesProcessor() );
+
 	options.order = options.order || [];
 	const rangeParser = new RangeParser( {
 		sameSelectionCharacters: options.sameSelectionCharacters
 	} );
-	const processor = new XmlDataProcessor( {
+	const processor = new XmlDataProcessor( viewDocument, {
 		namespaces: Object.keys( allowedTypes )
 	} );
 
@@ -927,7 +932,10 @@ class ViewStringify {
 function _convertViewElements( rootNode ) {
 	if ( rootNode.is( 'element' ) || rootNode.is( 'documentFragment' ) ) {
 		// Convert element or leave document fragment.
-		const convertedElement = rootNode.is( 'documentFragment' ) ? new ViewDocumentFragment() : _convertElement( rootNode );
+
+		const convertedElement = rootNode.is( 'documentFragment' ) ?
+			new ViewDocumentFragment( rootNode.document ) :
+			_convertElement( rootNode.document, rootNode );
 
 		// Convert all child nodes.
 		// Cache the nodes in array. Otherwise, we would skip some nodes because during iteration we move nodes
@@ -973,10 +981,10 @@ function _convertViewElements( rootNode ) {
 // module:engine/view/emptyelement~EmptyElement|module:engine/view/uielement~UIElement|
 // module:engine/view/containerelement~ContainerElement} A tree view
 // element converted according to its name.
-function _convertElement( viewElement ) {
+function _convertElement( viewDocument, viewElement ) {
 	const info = _convertElementNameAndInfo( viewElement );
 	const ElementConstructor = allowedTypes[ info.type ];
-	const newElement = ElementConstructor ? new ElementConstructor( info.name ) : new ViewElement( info.name );
+	const newElement = ElementConstructor ? new ElementConstructor( viewDocument, info.name ) : new ViewElement( viewDocument, info.name );
 
 	if ( newElement.is( 'attributeElement' ) ) {
 		if ( info.priority !== null ) {

src/model/model.js

@@ -334,7 +334,7 @@ export default class Model {
 	 *
 	 *		// You can create your own HtmlDataProcessor instance or use editor.data.processor
 	 *		// if you have not overridden the default one (which is the HtmlDataProcessor instance).
-	 *		const htmlDP = new HtmlDataProcessor();
+	 *		const htmlDP = new HtmlDataProcessor( viewDocument );
 	 *
 	 *		// Convert an HTML string to a view document fragment:
 	 *		const viewFragment = htmlDP.toView( htmlString );

src/model/node.js

@@ -189,20 +189,12 @@ export default class Node {
 	}
 
 	/**
-	 * {@link module:engine/model/document~Document Document} that owns this node or `null` if the node has no parent or is inside
-	 * a {@link module:engine/model/documentfragment~DocumentFragment DocumentFragment}.
+	 * Returns true if the node is in a tree rooted in the document (is a descendant of one of its roots).
 	 *
-	 * @readonly
-	 * @type {module:engine/model/document~Document|null}
+	 * @returns {Boolean}
 	 */
-	get document() {
-		// This is a top element of a sub-tree.
-		if ( this.root == this ) {
-			return null;
-		}
-
-		// Root may be `DocumentFragment` which does not have document property.
-		return this.root.document || null;
+	isAttached() {
+		return this.root.is( 'rootElement' );
 	}
 
 	/**