The translation editor destroys content. Any element outside the schema of its bundled CKEditor build is removed from the target value and written to the autosaved draft, with no warning, no console error, and an untouched source - so the loss is invisible in the workspace and surfaces, if ever, on the published translation.

extension/components/RichTextField.tsx builds a ClassicEditor from a fixed plugin list (Essentials, Paragraph, Bold, Italic, Underline, Link, List) with no GeneralHtmlSupport. CKEditor 5's data processor therefore drops every unknown element on load, and the stripped result is what onChange writes back.

The server side is not at fault: ApiTranslateController::editorFormats() resolves the real text format and sends its real toolbar, exactly as documented. The mismatch is that the client cannot honor what it is sent, and fails open, it discards the content rather than declining to edit it.

Steps to reproduce

  1. Give an SDC text/html prop a text format that permits more than the seven bundled plugins can express, e.g. one with media_embed, headings or tables. (We use the <a href="https://www.drupal.org/project/canvas_full_html">canvas_full_html</a> module; any rich format reproduces it.)
  2. On a Canvas page, place a component with that prop and author content using one of those capabilities - a media embed, an <h2>, a table.
  3. Open the page in Canvas Translate for a target language.
  4. Press the editor's own Copy source text into this field.
  5. The element is gone from the target, and the autosaved draft has it stripped.

Editing the field by hand reproduces it identically; opening the editor does not, because onChange does not fire on mount. So the trigger is a translator doing exactly what the UI invites.

Measured (2026-08-17, Chrome, Drupal 11.4.5)

On one page with a <drupal-media> PDF embed in a rich-text prop:

  • The API sends format: <canvas_full_html> and all 26 toolbar items.
  • The editor renders 8 buttons: Bold, Italic, Underline, Link, Bulleted List, Numbered List, Undo, Redo.
  • 14 items are dropped with toolbarview-item-unavailable: heading, strikethrough, superscript, subscript, removeFormat, alignment, indent, outdent, blockQuote, insertTable, drupalMedia, horizontalLine, specialCharacters, sourceEditing.
  • Source HTML contains <drupal-media …>; after copy-source the editor's data does not, and neither does the saved draft.

The dropped toolbar buttons are cosmetic. The deleted content is not.

On spec violation

The module's own content-translation capability lists source-fallback (no data loss) as a requirement (openspec/specs/content-translation/spec.md). An editor that silently discards markup breaks that requirement regardless of which plugins it happens to ship.

Proposed fix

Do not hand a value to an editor that cannot round-trip it. When a value contains markup outside the bundled build's representable set, report the row as plain text; FieldRow already renders a textarea for that case. The translator then edits raw HTML, plainer, but nothing is destroyed.

This is a small, self-contained server-side change and needs no bundle rebuild:

  • a helper that answers "can the bundled editor round-trip this value?", keyed on the element names the shipped plugin set can represent;
  • two call sites: ApiTranslateController::getTranslation() and ConfigTranslatableGlue::rows() - pass the row's source and target through it before emitting format.

Patch attached, happy to open it as an MR.

On the general case

Whatever the bundled editor eventually supports, it can never match every format a site configures (drupalMedia in particular is a Drupal-side plugin with no npm equivalent). So a guard of this shape is worth having permanently, not only until the plugin set grows. It fails closed instead of open, which is the property the spec requirement actually needs.

CommentFileSizeAuthor
#4 extension-editor-cannot-represent-media.patch8.83 KBdavituri
extension-editor-cannot-represent-media.patch6.78 KBdavituri

Issue fork canvas_translate-3617651

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

davituri created an issue. See original summary.

f0ns’s picture

f0ns commented

Tested the patch against 1.0.x. It applies cleanly, the premise checks out (RichTextField.tsx really does build only Essentials, Paragraph, Bold, Italic, Underline, Link, List, with no GeneralHtmlSupport), and reporting format: null is safe: the row's format only picks the widget, and the publish path writes #text only, so the stored text format survives a downgraded row. I wrote 20 unit cases against ExtensionEditorCapability::formatFor() and they all pass.

Two things that need attention:

1. The s tag is allowlisted but the bundle cannot represent it. REPRESENTABLE includes 's', but Strikethrough is a separate plugin and is not in PLUGINS (only Bold, Italic and Underline are). So a strikethrough element passes the guard, reaches the rich editor, and is dropped: exactly the bug this patch exists to prevent. Either drop 's' from the list or add Strikethrough to the build.

2. Attribute-level loss is not covered, and it is probably the more common case. Without GeneralHtmlSupport, CKEditor drops unknown attributes as well as unknown elements, but the guard only inspects tag names. Drupal's editor link plugin stores internal links like this:

<a href="/node/12" data-entity-type="node" data-entity-uuid="abc-123">See also</a>

The anchor tag is allowlisted, so the guard says "representable", the row goes to CKEditor, and the data-entity-type and data-entity-uuid attributes are stripped, quietly turning a managed entity reference into a bare href. Same for a class attribute on a paragraph, or target and rel on a link. On most sites that markup is far more widespread than media embeds, so the guard currently gives some false confidence.

Which raises a design question: is the allowlist the right primary fix? Adding GeneralHtmlSupport to the bundle would preserve unknown elements and attributes through the model, addressing the second point structurally rather than tag by tag. I do not think it is either/or, and I agree the guard should be permanent (no plugin set can match every site's format config, and drupalMedia has no npm equivalent). But shipping only the guard means accepting that class and data-* attributes keep disappearing silently.

Smaller notes: the b and i tags are allowlisted and semantically safe, but CKEditor normalises them to strong and em on load, so opening a row rewrites markup the translator never edited. The tag regex also matches inside attribute values and comments, which only costs an unnecessary textarea, so it fails in the safe direction. And the PHP allowlist has to track the TypeScript plugin list by hand with nothing to catch drift, which is how point 1 arose in the first place.

f0ns’s picture

f0ns commented
Status: Active » Needs work
davituri’s picture

davituri commented
StatusFileSize
new extension-editor-cannot-represent-media.patch8.83 KB

Thanks, both are fixed and the second one changed my mind about the shape of the guard.

1. s was allowlisted with no Strikethrough plugin behind it. Confirmed and removed.

2. Attribute-level loss. Also confirmed and you are right that it is the more common case — data-entity-type/data-entity-uuid on an internal link is written by editor's link dialog and losing it silently converts an entity reference into a bare path that breaks on the next alias change. target/rel and text-align-* go the same way.

What changed in the re-roll

The guard now parses the value and checks each element and its attributes against what the seven plugins actually produce. REPRESENTABLE became a map of element to permitted attributes; a keeps only href, since the build configures no link decorators.

That also removes the false positives you noted, where the regex matched <tag inside an attribute value or an HTML comment and stranded an editable row on a textarea.

Against 13 vectors covering every loss mode named in this issue, the parsing guard is correct on 13; the regex was correct on 7:

vector lossy? prev patch re-roll
paragraph / bold / italic / list / bare link no editor editor
<drupal-media> / <h2> / <table> / <blockquote> yes textarea textarea
<s> strikethrough yes editor ✗ textarea
<a data-entity-type data-entity-uuid> yes editor ✗ textarea
<a target rel> yes editor ✗ textarea
<p class="text-align-center"> yes editor ✗ textarea
tag name inside an attribute value no textarea ✗ editor
tag name inside a comment no textarea ✗ editor

On your b/i observation: I left them allowed and documented the line as normalisation is accepted where deletion is not. They are upcast to strong and em, which rewrites markup the translator never touched but does not change what renders. Sending every <b> to a raw textarea seemed a poor trade for that.

On the drift

PHP and TypeScript allowlists drift without coordination; point 1's inconsistency demonstrates this.

Agreed, the capability lives in RichTextField.tsx and the guard lives in PHP, so nothing can fail when they disagree. Two ways out, both better than the current patch:

  1. Move the guard client-side, into RichTextField next to PLUGINS — render a textarea instead of <CKEditor> when the value is unrepresentable. The two lists become one list in one file. I kept it server-side only because we consume this as a composer patch and cannot rebuild dist/index.global.js.
  2. Stop declaring the capability and measure it. Round-trip the value through the editor's own data processor and compare the element/attribute set of the output against the input; if anything was dropped, render the textarea. That needs no allowlist at all and stays correct for any future plugin set — which also answers the general case, since no fixed list can match every site's format. It needs a small normalisation allowance for b→strong and i→em.

I can write either, but that is a bigger change than this issue, and the architecture is your call.

f0ns’s picture

f0ns commented

I don't feel like the architecture is my call but the maintainer's call.

The changes look good.

One more thing I had as a comment on my issue as well by the maintainer:

This will need specs (cfr. https://www.drupal.org/project/canvas_translate/issues/3616274#comment-1...)

davituri’s picture

davituri commented

Right.
I'll put up an MR with the spec first, then the test cases against it.

Draft of the requirement, before we write tests against it:

Requirement: No silent loss of unrepresentable markup

The translation editor MUST NOT destroy markup it cannot round-trip. A prop value containing an element or attribute the bundled editor build cannot represent MUST be presented in a lossless widget instead, and MUST survive an edit-and-publish cycle unchanged except where the translator changed it.

  • Scenario: a value the editor cannot represent — WHEN a prop value contains &lt;drupal-media&gt;, a heading, a table, or a link carrying data-entity-type/data-entity-uuid, THEN the editor MUST NOT load it into a widget that would drop them, AND editing an adjacent field MUST NOT alter the value.
  • Scenario: a value the editor can represent — WHEN a value contains only markup the build round-trips, THEN it MUST be editable in the rich-text widget.
  • Scenario: normalisation is not loss — WHEN a value contains <b> or <i>, THEN upcasting to <strong>/<em> MUST NOT count as unrepresentable, since what renders is unchanged.

@lauriii - one question: The spec above holds whichever way it is implemented, so I'll carry the server-side guard from #4 unless you'd rather have one of:

  1. the guard client-side in RichTextField.tsx, next to PLUGINS, so the capability list exists once instead of twice;
  2. no list at all - round-trip the value through the editor's own data processor and compare the element/attribute set, which stays correct for any future plugin set,

GeneralHtmlSupport in the build shrinks what any guard has to catch but cannot replace it, since drupalMedia has no npm equivalent and would round-trip as inert markup a translator cannot see.

f0ns opened merge request !14

f0ns’s picture

f0ns commented

Opened an MR that takes a different route from the allowlist patch: rich text is now edited in the text format's own CKEditor 5 (Drupal core's, the same editor Canvas uses), loaded into the extension through a new editor-assets endpoint.

Media embeds, headings, tables and `data-entity-*` links are kept because the format's own plugins handle them, not because a guard catches them.

@davituri's option 2 from #4 is in as a safety net: each value is round-tripped through that editor before it is loaded, and anything it would drop is edited as HTML instead, unchanged. The spec from #6 is included, along with the adjacent-field scenario. Could you retest this?

E2E tests cover a real `` embed, a managed link and a heading through copy source → publish, plus the HTML fallback. Both fail on 1.0.x.

@lauriii since #6 left the architecture to you: this drops the bundled editor entirely (bundle 1.28 MB → 595 KB). Reasoning is in the MR description.

f0ns’s picture

f0ns commented
Status: Needs work » Needs review
f0ns’s picture

f0ns commented

lauriii made their first commit to this issue’s fork.

f0ns’s picture

f0ns commented

Reviewed the latest commits (74cfdad, 4ee0ffa). Agree with dropping the loss check: with the format's own editor, only markup the format disallows is dropped, same as in Canvas.

Locally on 4ee0ffa: type-check, build (dist in sync), Vitest 39/39, kernel 43/43, no new phpcs findings.

- The MR pipeline failed on an artifact upload error in the `composer` job (413), not on the code. This just needs a re-run.
- I'll update the MR description, which still mentions the loss check.

f0ns’s picture

f0ns commented

Pipeline is blocked by an unrelated CI failure (composer artifact too large, 413). Fix in #3628831: Composer CI job fails with 413: artifact too large; needs a rebase once that's in.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

We were missing logic to account for the fact that user may not have permission to use the text format. I also removed happy-dom which wasn't being used anymore since we got rid of the rich text loss tests. I think this should be ready now.

@f0ns do you want to check the most recent changes still?

  • lauriii committed e3310106 on 1.0.x authored by f0ns 
    fix: #3617651 Translating a rich-text prop silently deletes markup the...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Fixed

Thank you @f0ns and @davituri for the help with this! Merged! 🚀

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.