Skip to content

Conversation

@rrahir
Copy link
Collaborator

@rrahir rrahir commented Oct 14, 2025

The recent use of the property "plaintext-only" came with a change of behaviour in the composers, specifically in Firefox.

How to reproduce in FF:

  • write a single character in a grid composer
  • delete the character

-> a new line is inserted

Apparently, when setting the attribute contentEditable to plaintext-only, deleting the single character does not delete the span that encapsulates it if the latter has a class attribute. This couples with a recent refactoring of the content editable helper to handle new line characters and the empty composer is not detected as such

Task: 5082601

Description:

description of this task, what is implemented and why it is implemented that way.

Task: 5082601

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Collaborator

robodoo commented Oct 14, 2025

Pull request status dashboard

The recent use of the property "plaintext-only" came with a change of
behaviour in the composers, specifically in Firefox.

How to reproduce in FF:
- write a single character in a grid composer
- delete the character

-> a new line is inserted

Apparently, when setting the attribute `contentEditable` to
`plaintext-only`, deleting the single character does not delete the span
that encapsulates it if the latter has a class attribute. This couples
with a recent refactoring of the content editable helper to handle new
line characters and the empty composer is not detected as such

Task: 5082601
@rrahir rrahir force-pushed the saas-18.4-fix-contenteditable-plaintext-only-behaviour-rar branch from fd01c9d to f2d64d4 Compare October 15, 2025 13:55

function isEmptyParagraph(node: Node) {
return ["<br>", "<span><br></span>"].includes(
(node as HTMLElement).innerHTML.replaceAll(/\s(class|style)="[^"]*"/g, "")
Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't behave as expected if I type class="sdf" in a cell (plain text).

Also, it doesn't seem be problematic in this specific case, but as always, playing with innerHTML is dangerous. It doesn't trigger the security check in odoo, but it should if it was stricter

Any easy other way to achieve the same thing ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the problem arises with our own way of injecting stuff in the composer html,
theoretically, the other part of the fix (not adding empty classes) should fix the issue at hand but it quite fragile and someone else that works on the content helper (the upcoming dark theme PR might be a good candidate as it injects style AFAIK) might break it again. I will see if we can have a more solid comparison without crushing the performances

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the good solution would be to use innerText all the way but requires some polyfill for the tests, I'm not sure we will reach a "good" point where we don't end up testing the polyfill in the existing tests though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants