Skip to content

Commit

Permalink
[FIX] clipboard: don't open composer on firefox
Browse files Browse the repository at this point in the history
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 #2251

Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
LucasLefevre committed Mar 21, 2023
1 parent 7f01131 commit 1a66d74
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/components/composer/content_editable_helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/components/grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,10 @@ export class Grid extends Component<Props, SpreadsheetEnv> {
}

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
Expand Down
1 change: 1 addition & 0 deletions src/plugins/ui/edition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
12 changes: 12 additions & 0 deletions tests/components/composer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)");
Expand Down
6 changes: 6 additions & 0 deletions tests/plugins/edition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 1a66d74

Please sign in to comment.