Desktop: Fixes #14979: Fixed copying and cutting table columns in Rich Text editor

[bhaktideshmukh]

Problem

Previously, when working in the Rich Text editor, copying, cutting, or pasting isolated table columns behaved incorrectly

• Copying & Cutting: Successfully copied the data, but when attempting to delete the source selection, it only deleted the very first "anchor" cell and left the rest of the column completely untouched.
• Pasting: Pasting a copied column directly dumped the entire structure inside the single target cell instead of correctly splicing the columns sideways.

Solution

• This PR fixes issues with cutting, copying, and pasting table columns by letting the TinyMCE Table Plugin handle things naturally instead of forcing strict formatting rules.

• For cut and copy, when table cells are selected, we skip the custom override so the plugin can properly delete or copy full columns. We still capture the result afterward so Joplin can use it.

• For paste, if table content is detected, we avoid the usual Markdown conversion and instead use TinyMCE’s native paste, which keeps the column structure and spacing intact while still processing images correctly.

Fixes #14979 .

Testing

Existing Behavior

Recording.2026-04-15.194557.mp4

Current Behavior

Screen.Recording.2026-04-15.191709.mp4

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Fixes table column and multi-cell selection copy-cut operations in the rich text editor by detecting table context selections and routing them through native browser clipboard commands instead of standard content conversion handlers.

Changes

Cohort / File(s)	Summary
Table Selection Detection packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.ts, packages/app-desktop/gui/NoteEditor/utils/contextMenu.ts	Updated to identify multi-cell or multi-row table selections and emit 'table-selected' marker; context menu actions now route these selections to native execCommand('copy'/'cut') operations.
TinyMCE Editor Integration packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx	Enhanced copy and cut handlers to detect table context and schedule async clipboard reading for multi-cell selections; improved paste handling to bypass Markdown conversion when pasting table content within table boundaries; TypeScript spacing adjustments for type annotations.

Suggested labels

bug, desktop, editor

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Pr Description Must Follow Guidelines	❌ Error	PR description includes problem statement and solution explanation but lacks a dedicated Test Plan or verification steps section documenting reproduction steps and cross-platform validation.	Add a Test Plan section documenting: step-by-step reproduction instructions, expected behaviour after fix, cross-platform testing confirmation (Windows, macOS, Linux), and edge case verification.
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: fixing copying and cutting of table columns in the Rich Text editor, with specific issue reference.
Description check	✅ Passed	The description is directly related to the changeset, providing clear context on the problem, solution, and testing evidence for table column copy/cut/paste functionality.
Linked Issues check	✅ Passed	The code changes comprehensively address the requirements in #14979: handling multi-cell table selection for copy/cut operations and detecting table content for paste operations to preserve column structure.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing table column operations; minor TypeScript formatting adjustments are incidental to the core functionality improvements.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (3)

packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx (2)

1480-1487: The 50ms timeout is a potential race condition risk.

The arbitrary 50ms delay assumes the native copy completes and populates the clipboard within that time. On slower systems or under heavy load, this could fail silently (clipboard might not yet contain the expected content).

Consider:

1. Adding a brief comment explaining why 50ms was chosen.
2. Alternatively, increasing the timeout slightly for safety margin (e.g., 100ms).

The pattern itself is sound - letting TinyMCE's native handler populate the clipboard first, then re-copying with both text and HTML formats via copyHtmlToClipboard.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx` around
lines 1480 - 1487, The current setTimeout(…, 50) used after detecting selected
table cells (code around editor.dom.select('td…'), editor.dom.select('tr…')) is
a fragile race; replace the magic 50ms with a clearly named constant (e.g.,
CLIPBOARD_COPY_DELAY) and increase it to a safer value (e.g., 100–200ms) or make
it configurable, add a one-line comment explaining that this wait allows
TinyMCE's native copy to populate the clipboard before calling
clipboard.readHTML() and copyHtmlToClipboard(), and keep the same logic that
only triggers when multiple cells/rows are selected.

---

1480-1505: Consider extracting the table selection detection logic.

The same condition for detecting multi-cell/row table selection appears in three places:

1. useContextMenu.ts (lines 74-75)
2. onCopy (lines 1480-1481)
3. onCut (lines 1497-1498)

♻️ Suggested helper function

+function hasTableCellsOrRowsSelected(editor: Editor): boolean {
+	const selectedCells = editor.dom.select('td[data-mce-selected="1"], th[data-mce-selected="1"]');
+	return selectedCells.length > 1 || editor.dom.select('tr[data-mce-selected="1"]').length > 0;
+}

This could be placed in a utils file and reused across both TinyMCE.tsx and useContextMenu.ts.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx` around
lines 1480 - 1505, Extract the repeated multi-cell/row detection into a shared
helper (e.g., isTableMultiSelection(editor): boolean) placed in a utils file and
export it; replace the inline checks in TinyMCE.tsx (inside onCopy and onCut)
and in useContextMenu.ts with calls to this helper so all three locations use
the same logic (use editor.dom.select('td[data-mce-selected="1"],
th[data-mce-selected="1"]') and editor.dom.select('tr[data-mce-selected="1"]')
internally), update imports, and ensure behavior remains identical (including
the >1 check and any surrounding setTimeout logic).

packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.ts (1)

84-91: Consider using a more specific union type for the new event strings.

The type TinyMceEditorEvents|string is quite broad. For better type safety, consider defining specific string literals:

type ExecCommandEvent = 'execCommandCopy' | 'execCommandCut';

Then use TinyMceEditorEvents | ExecCommandEvent for the parameter type.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.ts`
around lines 84 - 91, The parameter type for fireEditorEvent is too broad
(TinyMceEditorEvents|string); define a narrow union for the new command strings
(e.g., type ExecCommandEvent = 'execCommandCopy' | 'execCommandCut') and change
the function signature to accept TinyMceEditorEvents | ExecCommandEvent so the
branches for execCommandCopy and execCommandCut are type-safe while preserving
existing TinyMceEditorEvents handling in fireEditorEvent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx`:
- Around line 1480-1487: The current setTimeout(…, 50) used after detecting
selected table cells (code around editor.dom.select('td…'),
editor.dom.select('tr…')) is a fragile race; replace the magic 50ms with a
clearly named constant (e.g., CLIPBOARD_COPY_DELAY) and increase it to a safer
value (e.g., 100–200ms) or make it configurable, add a one-line comment
explaining that this wait allows TinyMCE's native copy to populate the clipboard
before calling clipboard.readHTML() and copyHtmlToClipboard(), and keep the same
logic that only triggers when multiple cells/rows are selected.
- Around line 1480-1505: Extract the repeated multi-cell/row detection into a
shared helper (e.g., isTableMultiSelection(editor): boolean) placed in a utils
file and export it; replace the inline checks in TinyMCE.tsx (inside onCopy and
onCut) and in useContextMenu.ts with calls to this helper so all three locations
use the same logic (use editor.dom.select('td[data-mce-selected="1"],
th[data-mce-selected="1"]') and editor.dom.select('tr[data-mce-selected="1"]')
internally), update imports, and ensure behavior remains identical (including
the >1 check and any surrounding setTimeout logic).

In
`@packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.ts`:
- Around line 84-91: The parameter type for fireEditorEvent is too broad
(TinyMceEditorEvents|string); define a narrow union for the new command strings
(e.g., type ExecCommandEvent = 'execCommandCopy' | 'execCommandCut') and change
the function signature to accept TinyMceEditorEvents | ExecCommandEvent so the
branches for execCommandCopy and execCommandCut are type-safe while preserving
existing TinyMceEditorEvents handling in fireEditorEvent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 93bd15ce-0ee7-4aba-b5b1-d590e8d7545e

📥 Commits

Reviewing files that changed from the base of the PR and between cbdb3f1 and 7f0b1b0.

📒 Files selected for processing (3)

• packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx
• packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.ts
• packages/app-desktop/gui/NoteEditor/utils/contextMenu.ts

[bhaktideshmukh]

I have read the CLA Document and I hereby sign the CLA

[bhaktideshmukh]

@laurent22 Please review this PR, let me know if any changes are required. If not, please approve, so I'll merge this PR, thanks.
CC @f3tony

[bhaktideshmukh]

@laurent22 Whom should I reach out to review this PR?

[mrjo118]

You just need to wait and be patient

[laurent22]

Please see Claude's review:

Critical / Bugs

1. Race condition with 50ms setTimeout for clipboard read

In TinyMCE.tsx, both onCopy and onCut use setTimeout(() => { clipboard.readHTML(); ... }, 50) to read the clipboard after the native copy/cut completes. The 50ms delay is arbitrary — there's no guarantee the native clipboard write finishes in time. On slow machines, clipboard.readHTML() could return stale content.

2. Pasting into tables bypasses HTML sanitization

When inTable is true, processPastedHtml is called with null for both htmlToMarkdown and markupToHtml converters, skipping the Markdown round-trip sanitization. This could allow unsanitized HTML to be inserted when pasting into a table cell. TinyMCE's own content filtering should still apply, but this weakens the defense-in-depth.

3. ContextMenuOptions interface likely not updated

fireEditorEvent was changed to accept TinyMceEditorEvents|string in the implementation (useContextMenu.ts), but the interface definition in contextMenuUtils.ts (not in the PR) likely still types it as TinyMceEditorEvents. This could cause a type error or silently allow any string.

---

Medium — Design / Type Safety / Duplication

4. Duplicated table cell selection detection (3 times)

The same logic to detect multi-cell table selections is repeated 3 times:

const selectedCells = editor.dom.select('td[data-mce-selected="1"], th[data-mce-selected="1"]');
if (selectedCells.length > 1 || editor.dom.select('tr[data-mce-selected="1"]').length > 0) {

It appears in:

• TinyMCE.tsx — onCopy
• TinyMCE.tsx — onCut
• useContextMenu.ts — htmlToCopy

Additionally, onCopy and onCut share nearly identical bodies — both read the clipboard after a 50ms timeout and call copyHtmlToClipboard, with the only difference being onCut additionally calls onChangeHandler(). This should be extracted to:

• A hasTableCellSelection(editor) helper for the detection
• A shared handler for the copy-to-clipboard-after-timeout logic, parameterized by whether it's a cut

5. Magic string 'table-selected' as sentinel value

The literal string 'table-selected' flows through htmlToCopy across 3 files (useContextMenu.ts, contextMenu.ts x2). It should be extracted to a shared constant. While extremely unlikely, if a user's actual selection were exactly "table-selected", it would be misinterpreted.

6. Type widening — fireEditorEvent accepts any string

The parameter type was changed from TinyMceEditorEvents to TinyMceEditorEvents|string, which is too broad. Should be narrowed to TinyMceEditorEvents | 'execCommandCopy' | 'execCommandCut'.

7. document.execCommand('copy'/'cut') is deprecated

The PR introduces editor.getDoc().execCommand('copy') and execCommand('cut'). These are deprecated DOM APIs. They still work in Electron/Chromium, but worth noting for future maintenance.

---

Low — Style Violations

8. 12 unrelated whitespace changes in TinyMCE.tsx

The PR includes formatting-only changes that violate the project guideline "Do not make white space changes":

• Spaces added around | in type unions (e.g., HTMLDivElement|null → HTMLDivElement | null) — 3 instances
• Spaces added inside empty arrow functions (() => {} → () => { }) — 7 instances
• Tab replaced with space in a return statement — 1 instance
• Space added before /> in JSX self-closing tag — 1 instance

These should all be reverted.

---

Summary

The approach (delegating table operations to TinyMCE's native handling) is reasonable, but the implementation has some rough edges: the setTimeout race condition (#1), bypassed sanitization (#2), significant code duplication (#4), and the magic string sentinel (#5) are the main concerns. The whitespace changes (#8) should be dropped from the PR.