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

[FIX] clipboard: Fix clipboard cross-browser coverage #5063

Open
wants to merge 1 commit into
base: 18.0
Choose a base branch
from

Conversation

rrahir
Copy link
Collaborator

@rrahir rrahir commented Oct 8, 2024

The current implementation of the cross-browser clipboard relies heavily on an API that is currently only available on Chromium based browsers (see table).

There is currently no definitive announce that the feature will be adopted by other browsers (namely FF and Safari) and since the feature is still marked as experimental, it'd be better to rely on a more generic approach.

This revision changes the flow to rely entirely on the text/html mimetype as it is supported by all modern browsers.

Task: 4241877

Description:

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

Task: TASK_ID

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 8, 2024

Pull request status dashboard

@rrahir rrahir force-pushed the 18.0-fix-cross-browser-rar branch 2 times, most recently from 97b7c6c to 9f6b1ec Compare October 8, 2024 14:09
Copy link
Contributor

@hokolomopo hokolomopo left a comment

Choose a reason for hiding this comment

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

pouet pouet

src/helpers/clipboard/clipboard_helpers.ts Outdated Show resolved Hide resolved
src/helpers/clipboard/clipboard_helpers.ts Outdated Show resolved Hide resolved
src/plugins/ui_stateful/clipboard.ts Outdated Show resolved Hide resolved
tests/clipboard/clipboard_plugin.test.ts Outdated Show resolved Hide resolved
const oSheetClipboardData = htmlDocument
.querySelector("div")
?.getAttribute("data-osheet-clipboard");
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious: are there any pro/cons of parsing the HTML rather than using a regex ? And of putting the data in an attribute rather than a HTML comment ?

@rrahir rrahir force-pushed the 18.0-fix-cross-browser-rar branch 3 times, most recently from 36e3e98 to dd74a9a Compare October 14, 2024 07:33
The current implementation of the cross-browser clipboard relies heavily
on an API that is currently only available on Chromium based browsers
(see [table](https://developer.mozilla.org/en-US/docs/Web/API/Clipboard_API#api.clipboarditem)).

There is currently no definitive announce that the feature will be
adopted by other browsers (namely FF and Safari) and since the feature
is still marked as experimental, it'd be better to rely ona more generic
approach.

This revision changes the flow to rely entirely on the `text/html`
mimetype as it is supported by all modern browsers.

Task: 4241877
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