Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TINY-10892: Fix deleting from P with IMG into LI would create unwanted font-styles on Chrome #9638

Closed
wants to merge 11 commits into from
Closed
6 changes: 6 additions & 0 deletions .changes/unreleased/tinymce-TINY-10892-2024-05-12.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
project: tinymce
kind: Fixed
body: Deleting into a list from a paragraph that has an `img` tag could cause extra inline styles to be added
hamza0867 marked this conversation as resolved.
Show resolved Hide resolved
time: 2024-05-12T23:00:56.388965+01:00
custom:
Issue: TINY-10892
163 changes: 162 additions & 1 deletion modules/tinymce/src/core/main/ts/util/Quirks.ts
hamza0867 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Fun } from '@ephox/katamari';
import { Cell, Fun, Optional } from '@ephox/katamari';
import { SugarElement, Traverse } from '@ephox/sugar';

import Editor from '../api/Editor';
import Env from '../api/Env';
Expand Down Expand Up @@ -31,6 +32,7 @@ const Quirks = (editor: Editor): Quirks => {
const isWebKit = browser.isChromium() || browser.isSafari();
const isiOS = Env.deviceType.isiPhone() || Env.deviceType.isiPad();
const isMac = Env.os.isMacOS() || Env.os.isiOS();
const isChrome = browser.isChromium();

/**
* Executes a command with a specific state this can be to enable/disable browser editing features.
Expand Down Expand Up @@ -327,6 +329,160 @@ const Quirks = (editor: Editor): Quirks => {
});
};

// This helper function, deletes the content created by Chrome which has extra font-family style and replaces
// it with the original content saved on keydown which does not have font-family
const removeExtraFontFamilyOnKeyup = (editor: Editor, specialDelete: Cell<boolean>, content: Cell<Optional<string>>) => {
editor.on('keyup', (e) => {
if (isDefaultPrevented(e) || e.key !== 'Backspace' && e.key !== 'Delete' || !specialDelete.get()) {
return;
}

const rng = selection.getRng();
const container = rng.startContainer;
const root = dom.getRoot();
const parent = Optional.from(Traverse.parents(SugarElement.fromDom(container)).find((node) => node.dom.nodeName.toLowerCase() === 'li'));
hamza0867 marked this conversation as resolved.
Show resolved Hide resolved

let outsideContainer = container;
while (
outsideContainer.parentNode &&
outsideContainer.parentNode.firstChild === outsideContainer &&
outsideContainer.parentNode !== root &&
outsideContainer.parentNode.nodeName.toLowerCase() !== 'li'
) {
outsideContainer = outsideContainer.parentNode;
}
let outsideOffset = Optional.none<number>();
parent.each((parent) => {
parent.dom.childNodes.forEach((node, key) => {
if (node === outsideContainer) {
outsideOffset = Optional.some(key);
return;
}
});
outsideOffset.each((offset) => {
selection.getSel()?.setBaseAndExtent(parent.dom, offset + 1, parent.dom, parent.dom.childNodes.length);
editor.execCommand('Delete');
selection.setCursorLocation(parent.dom, offset + 1);
content.get().each((content) => editor.insertContent(content));
selection.setCursorLocation(parent.dom, offset + 1);
});

});

specialDelete.set(false);
});
};

/**
* Removes font-family style added when pressing backspace when the cursor is just before an image
* and there is a list before the image. #TINY-10892
*/
const removeExtraFontFamilyOnBackspace = () => {
const specialDelete = Cell(false);
const content = Cell<Optional<string>>(Optional.none());
hamza0867 marked this conversation as resolved.
Show resolved Hide resolved
editor.on('keydown', (e) => {
if (isDefaultPrevented(e) || e.key !== 'Backspace') {
return;
}

const rng = selection.getRng();
const container = rng.startContainer;
let parent = container;
const offset = rng.startOffset;
const root = dom.getRoot();

if (!rng.collapsed || offset !== 0) {
return;
}

while (
parent.parentNode &&
parent.parentNode.firstChild === parent &&
parent.parentNode !== root
) {
parent = parent.parentNode;
}
let hasImgNode = false;
parent.childNodes.forEach((node) => {
if (node.nodeName.toLowerCase() === 'img') {
hasImgNode = true;
return;
}
});
if (!hasImgNode || parent.previousSibling?.nodeName.toLowerCase() !== 'ol') {
hamza0867 marked this conversation as resolved.
Show resolved Hide resolved
return;
}

const bookmark = selection.getBookmark();

selection.select(parent, true);
content.set(Optional.some(selection.getContent()));
selection.bookmarkManager.moveToBookmark(bookmark);

specialDelete.set(true);
});

removeExtraFontFamilyOnKeyup(editor, specialDelete, content);

};

/**
* Removes font-family style added when pressing delete when the cursor is just before an image
* and there is a list before the image. #TINY-10892
*/
const removeExtraFontFamilyOnDelete = () => {
const specialDelete = Cell(false);
const content = Cell<Optional<string>>(Optional.none());
editor.on('keydown', (e) => {
if (isDefaultPrevented(e) || e.key !== 'Delete') {
return;
}

const rng = selection.getRng();
const container = rng.startContainer;
let parent = container;
const offset = rng.startOffset;
const root = dom.getRoot();

if (!rng.collapsed || offset !== container.textContent?.length) {
return;
}

while (
parent.parentNode &&
parent.parentNode.lastChild === parent &&
parent.parentNode !== root
) {
parent = parent.parentNode;
}
if (!parent.nextSibling) {
return;
}
parent = parent.nextSibling;
let hasImgNode = false;
parent.childNodes.forEach((node) => {
if (node.nodeName.toLowerCase() === 'img') {
hasImgNode = true;
return;
}
});
if (!hasImgNode || parent.previousSibling?.nodeName.toLowerCase() !== 'ol') {
return;
}

const bookmark = selection.getBookmark();

selection.select(parent, true);
content.set(Optional.some(selection.getContent()));
selection.bookmarkManager.moveToBookmark(bookmark);

specialDelete.set(true);
});

removeExtraFontFamilyOnKeyup(editor, specialDelete, content);

};

/**
* Removes a blockquote when backspace is pressed at the beginning of it.
*
Expand Down Expand Up @@ -690,6 +846,11 @@ const Quirks = (editor: Editor): Quirks => {
normalizeSelection();
}

if (isChrome) {
removeExtraFontFamilyOnBackspace();
removeExtraFontFamilyOnDelete();
}

// WebKit
if (isWebKit) {
documentElementEditingFocus();
Expand Down
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's no longer really a chrome test, nor it is a quirks test 🤔

I'm happy to keep the test, just rename it (and remember the describe call includes the name).

Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { RealKeys } from '@ephox/agar';
import { describe, it } from '@ephox/bedrock-client';
import { TinyAssertions, TinyHooks } from '@ephox/wrap-mcagar';

import Editor from 'tinymce/core/api/Editor';

describe('webdriver.tinymce.core.delete.QuirksChromeTest', () => {

const hook = TinyHooks.bddSetupLight<Editor>({
base_url: '/project/tinymce/js/tinymce'
}, [], true);

it('TINY-10892: Backspace from beginning of P that contains IMG into OL when the LI does NOT end with an inline element', async () => {
const editor = hook.editor();
editor.setContent(
`<ol>
<li>sdadsa</li>
</ol>
<p><em>sdada</em><img src="#" alt="" width="24" height="24"> dsada </p>`);
editor.selection.setCursorLocation(editor.getDoc().querySelector('em') as HTMLElement, 0);
await RealKeys.pSendKeysOn('iframe => body => em', [ RealKeys.backspace() ]);
TinyAssertions.assertContent(editor,
`<ol>
<li>sdadsa<em>sdada</em><img src="#" alt="" width="24" height="24"> dsada</li>
</ol>`
);
});

it('TINY-10892: Backspace from beginning of P that contains IMG into OL when the LI DOES end with an inline element', async () => {
const editor = hook.editor();
editor.setContent(
`<ol>
<li>sdadsa <strong>a</strong></li>
</ol>
<p><em>sdada</em><img src="#" alt="" width="24" height="24"> dsada </p>`);
editor.selection.setCursorLocation(editor.getDoc().querySelector('em') as HTMLElement, 0);
await RealKeys.pSendKeysOn('iframe => body => em', [ RealKeys.backspace() ]);
TinyAssertions.assertContent(editor,
`<ol>
<li>sdadsa <strong>a</strong><em>sdada</em><img src="#" alt="" width="24" height="24"> dsada</li>
</ol>`
);
});

it('TINY-10892: Delete from end of OL when the LI does NOT end with an inline element and has nextSibling P that contains IMG', async () => {
const editor = hook.editor();
editor.setContent(
`<ol>
<li>a</li>
</ol>
<p><em>sdada</em><img src="#" alt="" width="24" height="24"> dsada </p>`);
editor.selection.setCursorLocation(editor.getDoc().querySelector('li') as HTMLElement, 1);
await RealKeys.pSendKeysOn('iframe => body => li', [ RealKeys.text('\uE017') /* unicode for Delete key */]);
TinyAssertions.assertContent(editor,
`<ol>
<li>a<em>sdada</em><img src="#" alt="" width="24" height="24"> dsada</li>
</ol>`
);
});

it('TINY-10892: Delete from end of OL when the LI DOES end with an inline element and has nextSibling P that contains IMG', async () => {
const editor = hook.editor();
editor.setContent(
`<ol>
<li>a <strong>a</strong></li>
</ol>
<p><em>sdada</em><img src="#" alt="" width="24" height="24"> dsada </p>`);
editor.selection.setCursorLocation(editor.getDoc().querySelector('strong') as HTMLElement, 1);
await RealKeys.pSendKeysOn('iframe => body => strong', [ RealKeys.text('\uE017') /* unicode for Delete key */]);
TinyAssertions.assertContent(editor,
`<ol>
<li>a <strong>a</strong><em>sdada</em><img src="#" alt="" width="24" height="24"> dsada</li>
</ol>`
);
});

});