Skip to content

Commit

Permalink
TINY-9232: Parser needs to handle nested anchor cases better (#8258)
Browse files Browse the repository at this point in the history 
* TINY-9232: fixed so blocks are split if invalid blocks are inserted into anchors

* TINY-9232: more tests and refactoring

* TINY-9232: code cleanup
  • Loading branch information
@spocke
spocke committed Nov 18, 2022
1 parent 9aca3b7 commit 0cb16ae
Show file tree
Hide file tree
Showing 4 changed files with 208 additions and 64 deletions.
88 changes: 85 additions & 3 deletions modules/tinymce/src/core/main/ts/content/TransparentElements.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Arr, Obj, Type } from '@ephox/katamari';
import { Compare, SugarElement, SugarNode, Traverse } from '@ephox/sugar';
import { Compare, PredicateFilter, PredicateFind, SugarElement, SugarElements, SugarNode, Traverse } from '@ephox/sugar';

import AstNode from '../api/html/Node';
import Schema, { SchemaMap } from '../api/html/Schema';
import * as Empty from '../dom/Empty';
import * as NodeType from '../dom/NodeType';

export const transparentBlockAttr = 'data-mce-block';
Expand All @@ -20,16 +21,97 @@ const updateTransparent = (blocksSelector: string, transparent: Element) => {
if (transparent.getAttribute('data-mce-selected') === 'inline-boundary') {
transparent.removeAttribute('data-mce-selected');
}

return true;
} else {
transparent.removeAttribute(transparentBlockAttr);
return false;
}
};

export const updateChildren = (schema: Schema, scope: Element): void => {
const updateBlockStateOnChildren = (schema: Schema, scope: Element): Element[] => {
const transparentSelector = makeSelectorFromSchemaMap(schema.getTransparentElements());
const blocksSelector = makeSelectorFromSchemaMap(schema.getBlockElements());

Arr.each(scope.querySelectorAll(transparentSelector), (transparent) => updateTransparent(blocksSelector, transparent));
return Arr.filter(scope.querySelectorAll(transparentSelector), (transparent) => updateTransparent(blocksSelector, transparent));
};

const trimEdge = (el: DocumentFragment, leftSide: boolean) => {
const childPropertyName = leftSide ? 'lastChild' : 'firstChild';

for (let child = el[childPropertyName]; child; child = child[childPropertyName]) {
if (Empty.isEmpty(SugarElement.fromDom(child))) {
child.parentNode?.removeChild(child);
return;
}
}
};

const split = (parentElm: Element, splitElm: Node) => {
const range = document.createRange();
const parentNode = parentElm.parentNode;

if (parentNode) {
range.setStartBefore(parentElm);
range.setEndBefore(splitElm);
const beforeFragment = range.extractContents();
trimEdge(beforeFragment, true);

range.setStartAfter(splitElm);
range.setEndAfter(parentElm);
const afterFragment = range.extractContents();
trimEdge(afterFragment, false);

if (!Empty.isEmpty(SugarElement.fromDom(beforeFragment))) {
parentNode.insertBefore(beforeFragment, parentElm);
}

if (!Empty.isEmpty(SugarElement.fromDom(splitElm))) {
parentNode.insertBefore(splitElm, parentElm);
}

if (!Empty.isEmpty(SugarElement.fromDom(afterFragment))) {
parentNode.insertBefore(afterFragment, parentElm);
}

parentNode.removeChild(parentElm);
}
};

// This will find invalid blocks wrapped in anchors and split them out so for example
// <h1><a href="#"><h2>x</h2></a></h1> will find that h2 is invalid inside the H1 and split that out.
// This is a simplistic apporach so it's likely not covering all the cases but it's a start.
const splitInvalidChildren = (schema: Schema, scope: Element, transparentBlocks: Element[]): void => {
const blocksElements = schema.getBlockElements();
const rootNode = SugarElement.fromDom(scope);
const isBlock = (el: SugarElement) => SugarNode.name(el) in blocksElements;
const isRoot = (el: SugarElement) => Compare.eq(el, rootNode);

Arr.each(SugarElements.fromDom(transparentBlocks), (transparentBlock) => {
PredicateFind.ancestor(transparentBlock, isBlock, isRoot).each((parentBlock) => {
const invalidChildren = PredicateFilter.children(
transparentBlock,
(el) => isBlock(el) && !schema.isValidChild(SugarNode.name(parentBlock), SugarNode.name(el))
);

if (invalidChildren.length > 0) {
const stateScope = Traverse.parentElement(parentBlock);

Arr.each(invalidChildren, (child) => {
PredicateFind.ancestor(child, isBlock, isRoot).each((parentBlock) => {
split(parentBlock.dom as Element, child.dom);
});
});

stateScope.each((scope) => updateBlockStateOnChildren(schema, scope.dom));
}
});
});
};

export const updateChildren = (schema: Schema, scope: Element): void => {
const transparentBlocks = updateBlockStateOnChildren(schema, scope);
splitInvalidChildren(schema, scope, transparentBlocks);
};

export const updateElement = (schema: Schema, target: Element): void => {
Expand Down
9 changes: 9 additions & 0 deletions modules/tinymce/src/core/test/ts/browser/content/InsertContentTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -729,5 +729,14 @@ describe('browser.tinymce.core.content.InsertContentTest', () => {
TinyAssertions.assertContent(editor, '<div><a href="#1">a<p>b</p><strong><em>c</em></strong>d</a></div>');
TinyAssertions.assertContentPresence(editor, { 'a[data-mce-block]': 1 });
});

it('TINY-9232: Insert paragraphs in anchor inside paragraph should split the paragraph and anchor', () => {
const editor = hook.editor();

editor.setContent('<p><a href="#1">ad</a></p>');
TinySelections.setCursor(editor, [ 0, 0, 0 ], 1);
editor.insertContent('<p>b</p><p>c</p>');
TinyAssertions.assertContent(editor, '<p><a href="#1">a</a></p><p>b</p><p>c</p><p><a href="#1">d</a></p>');
});
});
});
138 changes: 77 additions & 61 deletions modules/tinymce/src/core/test/ts/browser/content/TransparentElementsTest.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { afterEach, beforeEach, context, describe, it } from '@ephox/bedrock-client';
import { Arr, Singleton } from '@ephox/katamari';
import { context, describe, it } from '@ephox/bedrock-client';
import { Arr } from '@ephox/katamari';
import { Hierarchy, Html, Insert, Remove, SugarBody, SugarElement, SugarNode } from '@ephox/sugar';
import { assert } from 'chai';

Expand All @@ -12,76 +12,92 @@ describe('browser.tinymce.core.content.TransparentElementsTest', () => {
const transparentElements = TransparentElements.elementNames(schema.getTransparentElements());
const textBlockElements = TransparentElements.elementNames(schema.getTextBlockElements());

context('update/updateCaret', () => {
const rootState = Singleton.value<SugarElement<HTMLElement>>();
it('TINY-9172: makeElementList', () => {
assert.deepEqual(TransparentElements.elementNames({ a: {}, h1: {}, A: {}, H1: {}}), [ 'a', 'h1' ]);
});

it('TINY-9230: hasBlockAttr', () => {
assert.isTrue(TransparentElements.hasBlockAttr(SugarElement.fromHtml<Element>('<a data-mce-block="true"></a>').dom));
assert.isFalse(TransparentElements.hasBlockAttr(SugarElement.fromHtml<Element>('<a></a>').dom));
});

beforeEach(() => {
context('updateChildren/updateCaret', () => {
const withScratchDiv = (html: string, f: (el: SugarElement<HTMLElement>) => void) => {
const root = SugarElement.fromTag('div');
Html.set(root, html);
Insert.append(SugarBody.body(), root);
rootState.set(root);
});

afterEach(() => {
rootState.get().each((root) => Remove.remove(root));
rootState.clear();
});

it('TINY-9172: makeElementList', () => {
assert.deepEqual(TransparentElements.elementNames({ a: {}, h1: {}, A: {}, H1: {}}), [ 'a', 'h1' ]);
});
f(root);
Remove.remove(root);
};

const testUpdateChildren = (testCase: { input: string; expected: string }) => {
withScratchDiv(testCase.input, (root) => {
TransparentElements.updateChildren(schema, root.dom);
assert.equal(Html.get(root), testCase.expected);
});
};

it('TINY-9230: hasBlockAttr', () => {
assert.isTrue(TransparentElements.hasBlockAttr(SugarElement.fromHtml<Element>('<a data-mce-block="true"></a>').dom));
assert.isFalse(TransparentElements.hasBlockAttr(SugarElement.fromHtml<Element>('<a></a>').dom));
});
const testUpdateCaret = (testCase: { input: string; path: number[]; expected: string }) => {
withScratchDiv(testCase.input, (root) => {
const scope = Hierarchy.follow(root, testCase.path).filter(SugarNode.isElement).getOrDie();
TransparentElements.updateCaret(schema, root.dom, scope.dom);
assert.equal(Html.get(root), testCase.expected);
});
};

it('TINY-9172: Should add data-mce-block on transparent elements if the contain blocks', () => {
const root = rootState.get().getOrDie();
const blockLinks = Arr.map(textBlockElements, (name) => `<a href="#"><${name}>link</${name}></a>`).join('');
const expectedBlockLinks = Arr.map(textBlockElements, (name) => `<a href="#" data-mce-block="true"><${name}>link</${name}></a>`).join('');

Html.set(root, `<a href="#">link</a><div>${blockLinks}</div>${blockLinks}<div><a href="#">link</a></div>`);
TransparentElements.updateChildren(schema, root.dom);
assert.equal(Html.get(root), `<a href="#">link</a><div>${expectedBlockLinks}</div>${expectedBlockLinks}<div><a href="#">link</a></div>`);
});

it('TINY-9172: Should add data-mce-block on transparent block elements that wrap blocks and also remove data-mce-selected="inline-boundary"', () => {
const root = rootState.get().getOrDie();

Html.set(root, '<div><a href="#" data-mce-selected="inline-boundary"><p>link</p></a></div>');
TransparentElements.updateChildren(schema, root.dom);
assert.equal(Html.get(root), '<div><a href="#" data-mce-block="true"><p>link</p></a></div>');
});

it('TINY-9172: Should update all anchors in element closest to the root only', () => {
const root = rootState.get().getOrDie();

Html.set(root, '<div><a href="#"><p>link</p></a><a href="#"><p>link</p></a></div><a href="#">not this</a><a href="#"><p>not this</p></a>');
const scope = Hierarchy.follow(root, [ 0, 0, 0 ]).filter(SugarNode.isElement).getOrDie();
TransparentElements.updateCaret(schema, root.dom, scope.dom);
assert.equal(
Html.get(root),
'<div><a href="#" data-mce-block="true"><p>link</p></a><a href="#" data-mce-block="true"><p>link</p></a></div><a href="#">not this</a><a href="#"><p>not this</p></a>'
);
});

it('TINY-9172: Should update anchor closest to root', () => {
const root = rootState.get().getOrDie();

Html.set(root, '<a href="#"><p>link</p></a>');
const scope = Hierarchy.follow(root, [ 0 ]).filter(SugarNode.isElement).getOrDie();
TransparentElements.updateCaret(schema, root.dom, scope.dom);
assert.equal(Html.get(root), '<a href="#" data-mce-block="true"><p>link</p></a>');
testUpdateChildren({
input: `<a href="#">link</a><div>${blockLinks}</div>${blockLinks}<div><a href="#">link</a></div>`,
expected: `<a href="#">link</a><div>${expectedBlockLinks}</div>${expectedBlockLinks}<div><a href="#">link</a></div>`
});
});

it('TINY-9172: Should update anchor even if target element is in another branch', () => {
const root = rootState.get().getOrDie();

Html.set(root, '<div><a href="#"><p>link</p></a><b><i>target</i></b></div>');
const scope = Hierarchy.follow(root, [ 0, 1, 0 ]).filter(SugarNode.isElement).getOrDie();
TransparentElements.updateCaret(schema, root.dom, scope.dom);
assert.equal(Html.get(root), '<div><a href="#" data-mce-block="true"><p>link</p></a><b><i>target</i></b></div>');
});
it('TINY-9172: Should add data-mce-block on transparent block elements that wrap blocks and also remove data-mce-selected="inline-boundary"', () => testUpdateChildren({
input: '<div><a href="#" data-mce-selected="inline-boundary"><p>link</p></a></div>',
expected: '<div><a href="#" data-mce-block="true"><p>link</p></a></div>'
}));

it('TINY-9232: Should split the H1 at the P element and remove any empty nodes that gets produced', () => testUpdateChildren({
input: '<h1><a href="#"><p>link</p></a></h1>',
expected: '<p>link</p>'
}));

it('TINY-9232: Should split the H1 at the P element and keep the contents that is around the paragraph', () => testUpdateChildren({
input: '<h1>a<a href="#"><p>b</p></a>c</h1>',
expected: '<h1>a</h1><p>b</p><h1>c</h1>'
}));

it('TINY-9232: Should split the H1 and the P element but not split the parent div element', () => testUpdateChildren({
input: '<div><h1>a<a href="#"><p>b</p></a>c</h1></div>',
expected: '<div><h1>a</h1><p>b</p><h1>c</h1></div>'
}));

it('TINY-9172: Should update all anchors in element closest to the root only', () => testUpdateCaret({
input: '<div><a href="#"><p>link</p></a><a href="#"><p>link</p></a></div><a href="#">not this</a><a href="#"><p>not this</p></a>',
path: [ 0, 0, 0 ],
expected: '<div><a href="#" data-mce-block="true"><p>link</p></a><a href="#" data-mce-block="true"><p>link</p></a></div><a href="#">not this</a><a href="#"><p>not this</p></a>'
}));

it('TINY-9172: Should update anchor closest to root', () => testUpdateCaret({
input: '<a href="#"><p>link</p></a>',
path: [ 0 ],
expected: '<a href="#" data-mce-block="true"><p>link</p></a>'
}));

it('TINY-9172: Should update anchor even if target element is in another branch', () => testUpdateCaret({
input: '<div><a href="#"><p>link</p></a><b><i>target</i></b></div>',
path: [ 0, 1, 0 ],
expected: '<div><a href="#" data-mce-block="true"><p>link</p></a><b><i>target</i></b></div>'
}));

it('TINY-9172: Split the H1 at the P point within the DIV but ignore the other P not in the caret scope', () => testUpdateCaret({
input: '<div><h1><a href="#"><p>link</p></a></h1></div><h1><a href="#"><p>link</p></a></h1>',
path: [ 0, 0, 0, 0 ],
expected: '<div><p>link</p></div><h1><a href="#"><p>link</p></a></h1>'
}));
});

context('isTransparentElementName', () => {
Expand Down
37 changes: 37 additions & 0 deletions modules/tinymce/src/core/test/ts/browser/html/DomParserTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1420,6 +1420,13 @@ describe('browser.tinymce.core.html.DomParserTest', () => {
context('Transparent elements', () => {
const getTransparentElements = (schema: Schema) => Arr.unique(Arr.map(Obj.keys(schema.getTransparentElements()), (s) => s.toLowerCase()));

const testSplitInvalidBlocksOut = (testCase: { input: string; expected: string }) => {
const parser = DomParser();
const serializedHtml = serializer.serialize(parser.parse(testCase.input));

assert.equal(serializedHtml, testCase.expected);
};

it('TINY-9172: inline transparents should not get data-mce-block attribute', () => {
const parser = DomParser();
const innerHtml = Arr.map(getTransparentElements(parser.schema), (name) => `<${name}>text</${name}>`).join('');
Expand Down Expand Up @@ -1448,6 +1455,36 @@ describe('browser.tinymce.core.html.DomParserTest', () => {

assert.equal(serializedHtml, expectedHtml);
});

it('TINY-9232: H1 in H1 should unwrap to single H1', () => testSplitInvalidBlocksOut({
input: '<h1><a href="#"><h1>foo</h1></a></h1>',
expected: '<h1>foo</h1>'
}));

it('TINY-9232: H1 and H2 in H1 should unwrap', () => testSplitInvalidBlocksOut({
input: '<h1><a href="#"><h1>a</h1><h2>b</h2></a></h1>',
expected: '<h1>a</h1><h2>b</h2>'
}));

it('TINY-9232: H1 and H2 in H1 should unwrap but text should remain links', () => testSplitInvalidBlocksOut({
input: '<h1><a href="#">a<h1>b</h1>c<h2>d</h2>e</a></h1>',
expected: '<h1><a href="#">a</a></h1><h1>b</h1><h1><a href="#">c</a></h1><h2>d</h2><h1><a href="#">e</a></h1>'
}));

it('TINY-9232: H1 in H1 in DIV should unwrap down to DIV', () => testSplitInvalidBlocksOut({
input: '<div>a<h1><a href="#"><h1>b</h1></a></h1>c</div>',
expected: '<div>a<h1>b</h1>c</div>'
}));

it('TINY-9232: Nested anchors wrapped in H1 and H2 should all unwrap', () => testSplitInvalidBlocksOut({
input: '<h1><a href="#1"><h2><a href="#2"><h3>foo</h3></a></h2></a></h1>',
expected: '<h3>foo</h3>'
}));

it('TINY-9232: H1 with content before and after anchor should be retained but the anchor should be unwrapped', () => testSplitInvalidBlocksOut({
input: '<h1>a<a href="#"><h1>foo</h1></a>b</h1>',
expected: '<h1>a</h1><h1>foo</h1><h1>b</h1>'
}));
});
});
});

0 comments on commit 0cb16ae

Please sign in to comment.