From 1a66d742fdf1e8bdda38f80d0f68eeda66de4422 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20Lef=C3=A8vre?= Date: Tue, 21 Mar 2023 15:57:28 +0000 Subject: [PATCH] [FIX] clipboard: don't open composer on firefox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Steps to reproduce: - open a spreadsheet with firefox - copy a cell - paste it somewhere else => the composer is open with the pasted content. Ok... that's weird, but not a big deal. But it gets better: - copy *multiple* cells - paste them somewhere else => boom The composer opens in the same way, but it doesn't support multiline text! This commit fixes the composer opening and it also makes the edition plugin more robust by removing any carriage return that could slip through. Additional small fix in the content editable helper, the selection `start` was not clipped if it was greater than the content length. Note: it doesn't happen on Chrome because `ev.data` is null when pasting content. It doesn't correctly implements the spec https://w3c.github.io/input-events/#dfn-data opw-3236303 closes odoo/o-spreadsheet#2251 Signed-off-by: RĂ©mi Rahir (rar) --- src/components/composer/content_editable_helper.ts | 1 + src/components/grid.ts | 4 ++++ src/plugins/ui/edition.ts | 1 + tests/components/composer.test.ts | 12 ++++++++++++ tests/plugins/edition.test.ts | 6 ++++++ 5 files changed, 24 insertions(+) diff --git a/src/components/composer/content_editable_helper.ts b/src/components/composer/content_editable_helper.ts index 7c983304dc..94a9c81b73 100644 --- a/src/components/composer/content_editable_helper.ts +++ b/src/components/composer/content_editable_helper.ts @@ -30,6 +30,7 @@ export class ContentEditableHelper { ); if (start < 0) start = 0; if (end > this.el!.textContent!.length) end = this.el!.textContent!.length; + if (start > this.el!.textContent!.length) start = this.el!.textContent!.length; } let startNode = this.findChildAtCharacterIndex(start); let endNode = this.findChildAtCharacterIndex(end); diff --git a/src/components/grid.ts b/src/components/grid.ts index d7f1f69fa1..54ada59c6b 100644 --- a/src/components/grid.ts +++ b/src/components/grid.ts @@ -832,6 +832,10 @@ export class Grid extends Component { } onInput(ev: InputEvent) { + // the user meant to paste in the sheet, not open the composer with the pasted content + if (!ev.isComposing && ev.inputType === "insertFromPaste") { + return; + } if (ev.data) { // if the user types a character on the grid, it means he wants to start composing the selected cell with that // character diff --git a/src/plugins/ui/edition.ts b/src/plugins/ui/edition.ts index 8f7cfb42e2..eeb3bf702e 100644 --- a/src/plugins/ui/edition.ts +++ b/src/plugins/ui/edition.ts @@ -418,6 +418,7 @@ export class EditionPlugin extends UIPlugin { } private setContent(text: string, selection?: ComposerSelection, raise?: boolean) { + text = text.replace(/[\r\n]/g, ""); const isNewCurrentContent = this.currentContent !== text; this.currentContent = text; diff --git a/tests/components/composer.test.ts b/tests/components/composer.test.ts index 6094a1b32f..86d3455bcb 100644 --- a/tests/components/composer.test.ts +++ b/tests/components/composer.test.ts @@ -575,6 +575,18 @@ describe("composer", () => { await nextTick(); expect(model.getters.getEditionMode()).toBe("inactive"); }); + test("input event triggered from a paste should not open composer", async () => { + gridInputEl.dispatchEvent( + new InputEvent("input", { + data: "d", + bubbles: true, + isComposing: false, + inputType: "insertFromPaste", + }) + ); + await nextTick(); + expect(model.getters.getEditionMode()).toBe("inactive"); + }); test("edit link cell changes the label", async () => { setCellContent(model, "A1", "[label](http://odoo.com)"); diff --git a/tests/plugins/edition.test.ts b/tests/plugins/edition.test.ts index d8e44fab47..5c5e575bfe 100644 --- a/tests/plugins/edition.test.ts +++ b/tests/plugins/edition.test.ts @@ -485,6 +485,12 @@ describe("edition", () => { expect(model.getters.getCurrentContent()).toBe("Hello from sheet1"); }); + test.each(["ABC\nDEF", "ABC\r\nDEF", "ABC\rDEF"])("carriage returns are cleaned", (text) => { + const model = new Model(); + model.dispatch("START_EDITION", { text }); + expect(model.getters.getCurrentContent()).toBe("ABCDEF"); + }); + test("select another cell which is empty set the content to an empty string", () => { const model = new Model(); setCellContent(model, "A1", "Hello sir");