Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #1826 from ckeditor/i/6091
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Reinmar authored Mar 3, 2020
2 parents 5fdb765 + fb8e77a commit 0e2f02e
Show file tree
Hide file tree
Showing 122 changed files with 1,873 additions and 1,661 deletions.
39 changes: 31 additions & 8 deletions src/controller/datacontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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.
*/
Expand Down
8 changes: 4 additions & 4 deletions src/controller/editingcontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 6 additions & 5 deletions src/conversion/downcasthelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
}
}
Expand Down Expand Up @@ -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 ) {
Expand Down Expand Up @@ -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 );
Expand Down
21 changes: 15 additions & 6 deletions src/conversion/viewconsumable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -239,6 +238,7 @@ export default class ViewConsumable {
*/
static consumablesFromElement( element ) {
const consumables = {
element,
name: true,
attributes: [],
classes: [],
Expand Down Expand Up @@ -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' ) ) {
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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 );
}
}
Expand Down Expand Up @@ -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 );
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/dataprocessor/htmldataprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions src/dataprocessor/xmldataprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions src/dev-utils/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 );

Expand All @@ -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() );
Expand Down
16 changes: 12 additions & 4 deletions src/dev-utils/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 = ']';
Expand Down Expand Up @@ -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 )
} );

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 ) {
Expand Down
2 changes: 1 addition & 1 deletion src/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down
16 changes: 4 additions & 12 deletions src/model/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
}

/**
Expand Down
Loading

0 comments on commit 0e2f02e

Please sign in to comment.