8
0
Krzysztof Krztoń 7 лет назад
Родитель
Сommit
fe65a2a5bd

+ 15 - 12
packages/ckeditor5-paste-from-office/package.json

@@ -7,24 +7,24 @@
     "ckeditor5",
     "ckeditor 5",
     "ckeditor5-feature",
-    "paste from Word",
-    "paste from Office"
+    "ckeditor5-plugin"
   ],
-  "dependencies": {},
-  "devDependencies": {
-    "@ckeditor/ckeditor5-basic-styles": "^10.0.3",
+  "dependencies": {
     "@ckeditor/ckeditor5-clipboard": "^10.0.3",
     "@ckeditor/ckeditor5-core": "^11.0.1",
+    "@ckeditor/ckeditor5-engine": "^11.0.0"
+  },
+  "devDependencies": {
+    "@ckeditor/ckeditor5-basic-styles": "^10.0.3",
     "@ckeditor/ckeditor5-editor-classic": "^11.0.1",
-    "@ckeditor/ckeditor5-engine": "^11.0.0",
     "@ckeditor/ckeditor5-enter": "^10.1.2",
-    "@ckeditor/ckeditor5-heading": "^10.0.3",
+    "@ckeditor/ckeditor5-heading": "^10.1.0",
     "@ckeditor/ckeditor5-link": "^10.0.4",
     "@ckeditor/ckeditor5-list": "^11.0.2",
     "@ckeditor/ckeditor5-paragraph": "^10.0.3",
     "@ckeditor/ckeditor5-table": "^11.0.0",
     "@ckeditor/ckeditor5-utils": "^11.0.0",
-    "eslint": "^4.15.0",
+    "eslint": "^5.5.0",
     "eslint-config-ckeditor5": "^1.0.7",
     "husky": "^0.14.3",
     "lint-staged": "^7.0.0"
@@ -36,13 +36,16 @@
   "author": "CKSource (http://cksource.com/)",
   "license": "GPL-2.0-or-later",
   "homepage": "https://ckeditor.com",
-  "bugs": {
-    "url": "https://github.com/ckeditor/ckeditor5-paste-from-office/issues"
-  },
+  "bugs": "https://github.com/ckeditor/ckeditor5-paste-from-office/issues",
   "repository": {
     "type": "git",
-    "url": "git+https://github.com/ckeditor/ckeditor5-paste-from-office.git"
+    "url": "https://github.com/ckeditor/ckeditor5-paste-from-office.git"
   },
+  "files": [
+    "lang",
+    "src",
+    "theme"
+  ],
   "scripts": {
     "lint": "eslint --quiet '**/*.js'",
     "precommit": "lint-staged"

+ 40 - 29
packages/ckeditor5-paste-from-office/src/filters/list.js

@@ -9,7 +9,6 @@
 
 import Element from '@ckeditor/ckeditor5-engine/src/view/element';
 import Matcher from '@ckeditor/ckeditor5-engine/src/view/matcher';
-import Range from '@ckeditor/ckeditor5-engine/src/view/range';
 import UpcastWriter from '@ckeditor/ckeditor5-engine/src/view/upcastwriter';
 
 /**
@@ -22,60 +21,65 @@ import UpcastWriter from '@ckeditor/ckeditor5-engine/src/view/upcastwriter';
  *
  * @param {module:engine/view/documentfragment~DocumentFragment} documentFragment The view structure which to transform.
  * @param {String} stylesString Styles from which list-like elements styling will be extracted.
+ * @param {module:engine/view/view~View} view
  */
-export function transformListItemLikeElementsIntoLists( documentFragment, stylesString ) {
+export function transformListItemLikeElementsIntoLists( documentFragment, stylesString, view ) {
 	if ( !documentFragment.childCount ) {
 		return;
 	}
 
-	const listLikeItems = findAllListItemLikeElements( documentFragment );
+	const itemLikeElements = findAllItemLikeElements( documentFragment, view );
 
-	if ( listLikeItems.length ) {
-		const writer = new UpcastWriter();
+	if ( !itemLikeElements.length ) {
+		return;
+	}
 
-		let currentList = null;
+	const writer = new UpcastWriter();
 
-		for ( let i = 0; i < listLikeItems.length; i++ ) {
-			if ( !currentList || listLikeItems[ i - 1 ].id !== listLikeItems[ i ].id ) {
-				const listStyle = findListStyle( listLikeItems[ i ], stylesString );
-				currentList = insertEmptyList( listStyle, listLikeItems[ i ].element, writer );
-			}
+	let currentList = null;
 
-			const listItem = transformElementIntoListItem( listLikeItems[ i ].element, writer );
+	itemLikeElements.forEach( ( itemLikeElement, i ) => {
+		if ( !currentList || isNewListNeeded( itemLikeElements[ i - 1 ], itemLikeElement ) ) {
+			const listStyle = detectListStyle( itemLikeElement, stylesString );
 
-			writer.appendChild( listItem, currentList );
+			currentList = insertNewEmptyList( listStyle, itemLikeElement.element, writer );
 		}
-	}
+
+		const listItem = transformElementIntoListItem( itemLikeElement.element, writer, view );
+
+		writer.appendChild( listItem, currentList );
+	} );
 }
 
 // Finds all list-like elements in a given document fragment.
 //
 // @param {module:engine/view/documentfragment~DocumentFragment} documentFragment Document fragment
 // in which to look for list-like nodes.
+// @param {module:engine/view/view~View} view
 // @returns {Array.<Object>} Array of found list-like items. Each item is an object containing:
 //
 //		* {module:engine/src/view/element~Element} element List-like element.
 //		* {Number} id List item id parsed from `mso-list` style (see `getListItemData()` function).
 //		* {Number} order List item creation order parsed from `mso-list` style (see `getListItemData()` function).
 //		* {Number} indent List item indentation level parsed from `mso-list` style (see `getListItemData()` function).
-function findAllListItemLikeElements( documentFragment ) {
-	const range = Range.createIn( documentFragment );
+function findAllItemLikeElements( documentFragment, view ) {
+	const range = view.createRangeIn( documentFragment );
 
 	// Matcher for finding list-like elements.
-	const listItemLikeElementsMatcher = new Matcher( {
+	const itemLikeElementsMatcher = new Matcher( {
 		name: /^p|h\d+$/,
 		styles: {
 			'mso-list': /.*/
 		}
 	} );
 
-	const listLikeItems = [];
+	const itemLikeElements = [];
 
 	for ( const value of range ) {
-		if ( value.type === 'elementStart' && listItemLikeElementsMatcher.match( value.item ) ) {
+		if ( value.type === 'elementStart' && itemLikeElementsMatcher.match( value.item ) ) {
 			const itemData = getListItemData( value.item );
 
-			listLikeItems.push( {
+			itemLikeElements.push( {
 				element: value.item,
 				id: itemData.id,
 				order: itemData.order,
@@ -84,7 +88,7 @@ function findAllListItemLikeElements( documentFragment ) {
 		}
 	}
 
-	return listLikeItems;
+	return itemLikeElements;
 }
 
 // Extracts list item style from the provided CSS.
@@ -101,14 +105,14 @@ function findAllListItemLikeElements( documentFragment ) {
 // and will be removed during CSS parsing.
 //
 // @param {Object} listLikeItem List-like item for which list style will be searched for. Usually
-// a result of `findAllListItemLikeElements()` function.
+// a result of `findAllItemLikeElements()` function.
 // @param {String} stylesString CSS stylesheet.
 // @returns {Object} result
 // @returns {String} result.type List type, could be `ul` or `ol`.
 // @returns {String} result.style List style, for example: `decimal`, `lower-roman`, etc. It is extracted
 // directly from Word stylesheet without further processing and may be not compatible
 // with CSS `list-style-type` property accepted values.
-function findListStyle( listLikeItem, stylesString ) {
+function detectListStyle( listLikeItem, stylesString ) {
 	const listStyleRegexp = new RegExp( `@list l${ listLikeItem.id }:level${ listLikeItem.indent }\\s*({[^}]*)`, 'gi' );
 	const listStyleTypeRegex = /mso-level-number-format:([^;]*);/gi;
 
@@ -132,11 +136,11 @@ function findListStyle( listLikeItem, stylesString ) {
 // Creates empty list of a given type and inserts it after a specified element.
 //
 // @param {Object} listStyle List style object which determines the type of newly created list.
-// Usually a result of `findListStyle()` function.
+// Usually a result of `detectListStyle()` function.
 // @param {module:engine/view/element~Element} element Element before which list is inserted.
 // @param {module:engine/view/upcastwriter~UpcastWriter} writer
 // @returns {module:engine/view/element~Element} Newly created list element.
-function insertEmptyList( listStyle, element, writer ) {
+function insertNewEmptyList( listStyle, element, writer ) {
 	const list = new Element( listStyle.type );
 	const position = element.parent.getChildIndex( element );
 
@@ -150,8 +154,10 @@ function insertEmptyList( listStyle, element, writer ) {
 //
 // @param {module:engine/view/element~Element} element Element which will be transformed into list item.
 // @param {module:engine/view/upcastwriter~UpcastWriter} writer
-function transformElementIntoListItem( element, writer ) {
-	removeBulletElement( element, writer );
+// @returns {module:engine/view/element~Element} New element to which the given one was transformed. It is
+// inserted in place of the old element (the reference to the old element is lost due to renaming).
+function transformElementIntoListItem( element, writer, view ) {
+	removeBulletElement( element, writer, view );
 
 	return writer.rename( 'li', element );
 }
@@ -188,7 +194,8 @@ function getListItemData( element ) {
 //
 // @param {module:engine/view/element~Element} element
 // @param {module:engine/view/upcastwriter~UpcastWriter} writer
-function removeBulletElement( element, writer ) {
+// @param {module:engine/view/view~View} view
+function removeBulletElement( element, writer, view ) {
 	// Matcher for finding `span` elements holding lists numbering/bullets.
 	const bulletMatcher = new Matcher( {
 		name: 'span',
@@ -197,7 +204,7 @@ function removeBulletElement( element, writer ) {
 		}
 	} );
 
-	const range = Range.createIn( element );
+	const range = view.createRangeIn( element );
 
 	for ( const value of range ) {
 		if ( value.type === 'elementStart' && bulletMatcher.match( value.item ) ) {
@@ -205,3 +212,7 @@ function removeBulletElement( element, writer ) {
 		}
 	}
 }
+
+function isNewListNeeded( previousItem, currentItem ) {
+	return previousItem.id !== currentItem.id;
+}

+ 4 - 2
packages/ckeditor5-paste-from-office/src/filters/utils.js

@@ -141,8 +141,10 @@ function normalizeSafariSpaceSpans( htmlString ) {
 // @param {Document} htmlDocument Native `Document` object in which spacing should be normalized.
 function normalizeSpacerunSpans( htmlDocument ) {
 	htmlDocument.querySelectorAll( 'span[style*=spacerun]' ).forEach( el => {
-		const spacesReplacemanet = Array( el.innerText.length + 1 ).join( '\u00A0 ' ).substr( 0, el.innerText.length );
+		// Use `el.childNodes[ 0 ].data.length` instead of `el.innerText.length`. For `el.innerText.length` which
+		// contains spaces mixed with `&nbsp;` Edge browser returns incorrect length.
+		const innerTextLength = el.childNodes[ 0 ].data.length;
 
-		el.innerHTML = spacesReplacemanet;
+		el.innerHTML = Array( innerTextLength + 1 ).join( '\u00A0 ' ).substr( 0, innerTextLength );
 	} );
 }

+ 1 - 1
packages/ckeditor5-paste-from-office/src/pastefromoffice.js

@@ -61,7 +61,7 @@ export default class PasteFromOffice extends Plugin {
 	_normalizeWordInput( input, dataTransfer, model ) {
 		const { body, stylesString } = parseHtml( input );
 
-		transformListItemLikeElementsIntoLists( body, stylesString );
+		transformListItemLikeElementsIntoLists( body, stylesString, this.editor.editing.view );
 		replaceImagesSourceWithBase64( body, dataTransfer.getData( 'text/rtf' ), model );
 
 		return body;

+ 221 - 0
packages/ckeditor5-paste-from-office/tests/README.md

@@ -0,0 +1,221 @@
+# Normalization and integration testing in `Paste from Office`
+
+To test if content pasted from any external application is transformed correctly by the editor the test itself needs to
+use data which is put into the native clipboard by the application. For test purpose, such data is stored in the single file
+called `fixture` file.
+
+The `fixture` file is usually a HTML file containing HTML content which was fetched from the native browser `dataTransfer`
+object (`dataTransfer.getData( 'html/text' )`) when content was pasted to the browser. This ensures that `fixture`
+file provides exactly same data as a real use scenario.
+
+## Fixture files
+
+_For example files see `_data/basic-style/bold-within-text/`_.
+
+The `fixture` files are grouped per feature (which usually corresponds to editor plugins, for example `basic-styles`, `list`, etc).
+All fixtures are stored in `_data/feature-name/` directory (for example `_data/basic-style/`). Each feature (which
+will be called **group**) has a separate folder per fixture. Each fixture is used to create one normalization and one integration test.
+
+Each fixture folder contains:
+
+- original input file - `bold-with-text.docx`
+- input fixture - `input.word2016.html`
+- normalized output fixture - `normalized.word2016.html`
+- model output fixture - `model.word2016.html`
+
+In some cases, different browsers produces different input data. For such situations, additional fixtures are stored.
+For example if input data is different for Safari, additional `input.safari.word2016.html` file will be present in fixture directory.
+
+## Tests group index
+
+_For example file see `_data/basic-style/index.js`_.
+
+Each group of fixtures contains index file (`index.js` in group folder e.g. `_data/basic-styles/index.js`).
+Its purpose is to simply import all fixture files from the group and expose them for further use. Index file has the following structure:
+
+
+```
+// Import default/generic fixtures.
+// Input fixtures.
+import boldWithinText from './bold-within-text/input.word2016.html';
+
+// Expected normalized fixtures.
+import boldWithinTextNormalized from './bold-within-text/normalized.word2016.html';
+
+// Expected model fixtures.
+import boldWithinTextModel from './bold-within-text/model.word2016.html';
+
+// Export imported generic fixtures for future use.
+export const fixtures = {
+    input: {
+        boldWithinText: boldWithinText
+    },
+    normalized: {
+        boldWithinText: boldWithinTextNormalized
+    },
+    model: {
+        boldWithinText: boldWithinTextModel
+    }
+}
+```
+
+Such structure exports generic fixtures (the ones which are the same for more than one browser and will be used if no browser specific fixtures are present).
+
+Index files must also export browser specific fixtures. In the simplest case if there are none, it exports empty object:
+
+
+```
+export browserFixtures = {};
+```
+
+If there are any browser specific fixtures, they are exported in a similar manner to generic ones (apart from being grouped by a browser):
+
+
+```
+// Export imported browser-specific fixtures for future use.
+export const browserFixtures = {
+    safari: {
+        input: {
+            boldWithinText: boldWithinTextSafari
+        },
+        normalized: {
+            boldWithinText: boldWithinTextNormalizedSafari
+        },
+        model: {
+            boldWithinText: boldWithinTextModelSafari
+        }
+    }
+}
+```
+
+### What if only input or one of the expected output fixtures are different for specific browser? Could fixtures be mixed?
+
+There are cases when only some fixtures differ for a given browser. In such cases browser fixtures export reuses generic fixtures:
+
+```
+// Export imported browser-specific fixtures for future use.
+export const browserFixtures = {
+    safari: {
+        input: {
+            boldWithinText: boldWithinText // generic
+        },
+        normalized: {
+            boldWithinText: boldWithinTextNormalizedSafari // Safari specific
+        },
+        model: {
+            boldWithinText: boldWithinTextModel // generic
+        }
+    }
+}
+```
+
+## Fixtures aggregation
+
+_See `_utils/fixtures.js`_.
+
+All group indexes files are aggregated in the `fixtures` util (`_utils/fixtures.js`) and exposed for tests in a single
+`fixtures` and `browserFixtures` objects:
+
+
+```
+// Import fixtures.
+import { fixtures as basicStyles, browserFixtures as basicStylesBrowser } from '../_data/basic-styles/index.js';
+
+// Generic fixtures.
+export const fixtures = {
+	'basic-styles': basicStyles
+};
+
+// Browser specific fixtures.
+export const browserFixtures = {
+	'basic-styles': basicStylesBrowser
+};
+```
+
+## Tests generation
+
+_See `data/normalization.js` and `data/integration.js`_.
+
+Tests based on fixture files are generated by the special util function `generateTests()` (see `_utils/utils.js`). This function
+is specifically designed to generate `normalization` (see `data/normalization.js`) or `integration` (see `data/integration.js`)
+tests using provided fixtures group, for example:
+
+
+```
+generateTests( {
+	input: 'basic-styles', // Group name.
+	type: 'integration', // Tests type (integration or normalization).
+	browsers: [ 'chrome', 'firefox', 'safari', 'edge' ], // For which browsers generate tests.
+	editorConfig: { // Editor config which will be used during editor creation which is used in tests.
+		plugins: [ Clipboard, Paragraph, Heading, Bold, Italic, Underline, Strikethrough, PasteFromOffice ]
+	},
+	skip: { // Names of fixtures which tests should be skipped (object `key` is the name of the browser for which to skip tests).
+		safari: [ 'italicStartingText', 'multipleStylesSingleLine', 'multipleStylesMultiline' ] // Skip due to spacing issue (#13).
+	}
+} );
+```
+
+## Adding new tests
+
+### To an existing group
+
+1. Create new fixtures directory in a group to which you plan to add tests (e.g. `_data/link/new-use-case/`).
+2. Add all necessary fixture files to the above directory:
+	* original input file - `new-use-case.docx`
+	* input fixture - `input.word2016.html` (to acquire clipboard data you may use `integration.html` manual test which prints `text/html` on paste)
+	* normalized output fixture - `normalized.word2016.html`
+	* model output fixture - `model.word2016.html`
+	* any browser specific fixtures
+3. Add new fixtures to group `index.js` file.
+
+That's all, added fixtures will be now used to generate normalization and integration test.
+
+### To a new group
+
+1. Create new group directory, for example `_data/new-group/`.
+2. Create new fixtures directories (one per input fixture file) in `_data/new-group/`, each containing:
+	* original input file - `new-use-case.docx`
+	* input fixture - `input.word2016.html` (to acquire clipboard data you may use `integration.html` manual test which prints `text/html` on paste)
+	* normalized output fixture - `normalized.word2016.html`
+	* model output fixture - `model.word2016.html`
+	* any browser specific fixtures
+3. Create group `index.js` file (`_data/new-group/index.js`) importing all necessary fixtures.
+4. Add new group to fixtures util `_utils/fixtures.js`:
+
+
+```
+// Import fixtures.
+import { fixtures as newGroup, browserFixtures as newGroupBrowser } from '../_data/new-group/index.js';
+
+// Generic fixtures.
+export const fixtures = {
+	'new-group': newGroup
+};
+
+// Browser specific fixtures.
+export const browserFixtures = {
+	'new-group': newGroupBrowser
+};
+```
+
+5. Add `generateTests()` function call in `data/normalization.js` to generate normalization and in `data/integration.js`
+to generate integration tests:
+
+```
+// normalization.js
+generateTests( {
+	input: 'new-group',
+	type: 'normalization',
+	browsers: [ 'chrome', 'firefox', 'safari', 'edge' ]
+	editorConfig: { ... }
+} );
+
+// integration.js
+generateTests( {
+	input: 'new-group',
+	type: 'integration',
+	browsers: [ 'chrome', 'firefox', 'safari', 'edge' ]
+	editorConfig: { ... }
+} );
+
+```

+ 6 - 5
packages/ckeditor5-paste-from-office/tests/filters/list.js

@@ -5,6 +5,7 @@
 
 import HtmlDataProcessor from '@ckeditor/ckeditor5-engine/src/dataprocessor/htmldataprocessor';
 import { stringify } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
+import View from '@ckeditor/ckeditor5-engine/src/view/view';
 
 import { transformListItemLikeElementsIntoLists } from '../../src/filters/list';
 
@@ -17,7 +18,7 @@ describe( 'Filters', () => {
 				const html = '<p style="mso-list:l0 level1 lfo0"><span style="mso-list:Ignore">1.</span>Item 1</p>';
 				const view = htmlDataProcessor.toView( html );
 
-				transformListItemLikeElementsIntoLists( view, '' );
+				transformListItemLikeElementsIntoLists( view, '', new View() );
 
 				expect( view.childCount ).to.equal( 1 );
 				expect( view.getChild( 0 ).name ).to.equal( 'ol' );
@@ -28,7 +29,7 @@ describe( 'Filters', () => {
 				const html = '<p style="mso-list:l0 level1 lfo0"><span style="mso-list:Ignore">1.</span>Item 1</p>';
 				const view = htmlDataProcessor.toView( html );
 
-				transformListItemLikeElementsIntoLists( view, '@list l0:level1 { mso-level-number-format: bullet; }' );
+				transformListItemLikeElementsIntoLists( view, '@list l0:level1 { mso-level-number-format: bullet; }', new View() );
 
 				expect( view.childCount ).to.equal( 1 );
 				expect( view.getChild( 0 ).name ).to.equal( 'ul' );
@@ -39,7 +40,7 @@ describe( 'Filters', () => {
 				const html = '<h1>H1</h1><p>Foo Bar</p>';
 				const view = htmlDataProcessor.toView( html );
 
-				transformListItemLikeElementsIntoLists( view, '' );
+				transformListItemLikeElementsIntoLists( view, '', new View() );
 
 				expect( view.childCount ).to.equal( 2 );
 				expect( stringify( view ) ).to.equal( html );
@@ -49,7 +50,7 @@ describe( 'Filters', () => {
 				const html = '<p style="mso-list:"><span style="mso-list:Ignore">1.</span>Item 1</p>';
 				const view = htmlDataProcessor.toView( html );
 
-				transformListItemLikeElementsIntoLists( view, '' );
+				transformListItemLikeElementsIntoLists( view, '', new View() );
 
 				expect( view.childCount ).to.equal( 1 );
 				expect( view.getChild( 0 ).name ).to.equal( 'ol' );
@@ -59,7 +60,7 @@ describe( 'Filters', () => {
 			it( 'handles empty body correctly', () => {
 				const view = htmlDataProcessor.toView( '' );
 
-				transformListItemLikeElementsIntoLists( view, '' );
+				transformListItemLikeElementsIntoLists( view, '', new View() );
 
 				expect( view.childCount ).to.equal( 0 );
 				expect( stringify( view ) ).to.equal( '' );