Skip to content

Commit

Permalink
[FIX] BottomBarSheet: Rename a sheet with style content on Firefox
Browse files Browse the repository at this point in the history
The issue addressed in PR #3351 unfortunately used a
`contentEditable` value that is not compatible with Firefox [1]

This revision implements the first strategy explored by the
aforementioned PR by removing the full format from  the contenteditable
span when we stop the edition.

Testing required to define some properties as JSdom does not properly
support the implementation of innerText (see [2])

[1]: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/contenteditable#browser_compatibility
[2]: jsdom/jsdom#1245

closes #3922

Task: 3754944
X-original-commit: ed7d3fa
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
  • Loading branch information
rrahir committed Mar 25, 2024
1 parent e0580b0 commit 1561d0a
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 5 deletions.
7 changes: 5 additions & 2 deletions src/components/bottom_bar_sheet/bottom_bar_sheet.ts
Expand Up @@ -161,13 +161,16 @@ export class BottomBarSheet extends Component<Props, SpreadsheetChildEnv> {
}

private stopEdition() {
if (!this.state.isEditing) return;
const input = this.sheetNameRef.el;
if (!this.state.isEditing || !input) return;

this.state.isEditing = false;
this.editionState = "initializing";
this.sheetNameRef.el?.blur();
input.blur();

const inputValue = this.getInputContent() || "";
input.innerText = inputValue;

interactiveRenameSheet(this.env, this.props.sheetId, inputValue, () => this.startEdition());
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/bottom_bar_sheet/bottom_bar_sheet.xml
Expand Up @@ -19,7 +19,7 @@
t-on-dblclick="() => this.onDblClick()"
t-on-focusout="() => this.onFocusOut()"
t-on-keydown="(ev) => this.onKeyDown(ev)"
t-att-contenteditable="state.isEditing ? 'plaintext-only': 'false'"
t-att-contenteditable="state.isEditing ? 'true': 'false'"
/>
<span class="o-sheet-icon ms-1" t-on-click.stop="(ev) => this.onIconClick(ev)">
<t t-call="o-spreadsheet-Icon.TRIANGLE_DOWN"/>
Expand Down
21 changes: 19 additions & 2 deletions tests/bottom_bar/bottom_bar_component.test.ts
Expand Up @@ -254,7 +254,8 @@ describe("BottomBar component", () => {
const sheetName = fixture.querySelector<HTMLElement>(".o-sheet-name")!;
expect(sheetName.getAttribute("contenteditable")).toEqual("false");
await doubleClick(sheetName);
expect(sheetName.getAttribute("contenteditable")).toEqual("plaintext-only");
await nextTick();
expect(sheetName.getAttribute("contenteditable")).toEqual("true");
expect(document.activeElement).toEqual(sheetName);
});

Expand All @@ -273,7 +274,7 @@ describe("BottomBar component", () => {
await nextTick();
await click(fixture, ".o-menu-item[data-name='rename'");
const sheetName = fixture.querySelector<HTMLElement>(".o-sheet-name")!;
expect(sheetName.getAttribute("contenteditable")).toEqual("plaintext-only");
expect(sheetName.getAttribute("contenteditable")).toEqual("true");
expect(document.activeElement).toEqual(sheetName);
});

Expand Down Expand Up @@ -338,6 +339,22 @@ describe("BottomBar component", () => {
expect(raiseError).not.toHaveBeenCalled();
expect(sheetName.textContent).toEqual("SHEET1");
});

test("Pasting styled content in sheet name and renaming sheet does not throw a trackback", async () => {
const HTML = `<span style="color: rgb(242, 44, 61); background-color: rgb(0, 0, 0);">HELLO</span>`;

const sheetName = fixture.querySelector<HTMLElement>(".o-sheet-name")!;
triggerMouseEvent(sheetName, "dblclick");
await nextTick();

sheetName.innerHTML = HTML;
await keyDown({ key: "Enter" });

expect(sheetName.getAttribute("contenteditable")).toEqual("false");
await nextTick();

expect(sheetName.innerText).toEqual("HELLO");
});
});

test("Can't rename a sheet in readonly mode", async () => {
Expand Down
9 changes: 9 additions & 0 deletions tests/setup/jest.setup.ts
Expand Up @@ -23,6 +23,15 @@ beforeAll(() => {
(str, ...values) => str,
() => true
);
Object.defineProperty(Element.prototype, "innerText", {
get: function () {
return this.textContent;
},
set: function (value) {
this.textContent = value;
this.innerHTML = value;
},
});
});

beforeEach(() => {
Expand Down

0 comments on commit 1561d0a

Please sign in to comment.