From 34b0a7c2c8f07472dd9ac87d54e8c1b825bd9e11 Mon Sep 17 00:00:00 2001 From: kliesch Date: Tue, 16 Jun 2026 15:42:15 +0200 Subject: [PATCH] Dnd of Content Images should not ne converted to imageBlock elements --- .../src/ContentImageEditingPlugin.ts | 96 ++++++++++++++++++- .../src/converters.ts | 71 ++++++++++++++ 2 files changed, 166 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-coremedia-images/src/ContentImageEditingPlugin.ts b/packages/ckeditor5-coremedia-images/src/ContentImageEditingPlugin.ts index 79e2f08807..8370cc52f0 100644 --- a/packages/ckeditor5-coremedia-images/src/ContentImageEditingPlugin.ts +++ b/packages/ckeditor5-coremedia-images/src/ContentImageEditingPlugin.ts @@ -6,7 +6,7 @@ import { getOptionalPlugin, reportInitEnd, reportInitStart } from "@coremedia/ck import type { Logger } from "@coremedia/ckeditor5-logging"; import { LoggerProvider } from "@coremedia/ckeditor5-logging"; import ModelBoundSubscriptionPlugin from "./ModelBoundSubscriptionPlugin"; -import { editingDowncastXlinkHref, preventUpcastImageSrc } from "./converters"; +import { editingDowncastXlinkHref, preventUpcastImageSrc, upcastContentImageAsInline } from "./converters"; import { openImageInTabCommandName, registerOpenImageInTabCommand, @@ -62,6 +62,20 @@ export default class ContentImageEditingPlugin extends Plugin { // src attribute for the editing view asynchronously. // If not prevented, the src-attribute from GRS would be written to the model. this.editor.conversion.for("upcast").add(preventUpcastImageSrc()); + + // Force content images to become imageInline when dropped inside a + // paragraph (inline context). This prevents ImageBlockEditing from + // splitting the host paragraph. Runs at high priority, before + // ImageBlockEditing's normal-priority converter. + this.editor.conversion.for("upcast").add(upcastContentImageAsInline()); + + // When a content image is dragged and dropped between block elements, + // CKEditor's image upcasting may create imageBlock instead of imageInline + // (because the drop target is a block-level position). The post-fixer + // registered here converts any such imageBlock back to a paragraph + // containing an imageInline, which is the correct model representation + // for CoreMedia content images. + ContentImageEditingPlugin.#setupImageBlockToInlinePostFixer(this.editor); } static #setupXlinkHrefConversion(editor: Editor, modelAttributeName: string, dataAttributeName: string): void { @@ -103,6 +117,86 @@ export default class ContentImageEditingPlugin extends Plugin { .add(editingDowncastXlinkHref(editor, modelElementName, ContentImageEditingPlugin.#logger)); } + /** + * Ensures that content images dropped between block elements remain as + * `imageInline` in the model. + * + * When dragging a content image and dropping it between two paragraphs, + * CKEditor's image upcast pipeline detects a block-level drop position and + * creates an `imageBlock` model element instead of `imageInline`. Because + * `xlink-href` was not allowed on `imageBlock`, the attribute was silently + * dropped and GHS captured `data-xlink-href` into `htmlImgAttributes`, + * making the image disappear from the editing view. + * + * This method: + * 1. Extends the `imageBlock` schema to allow `xlink-href` so that the + * existing `attributeToAttribute` upcast converter sets the attribute + * correctly (and GHS no longer captures it). + * 2. Registers a model post-fixer that converts every `imageBlock` carrying + * a `xlink-href` attribute into a `` containing an + * `imageInline`, which is the correct CoreMedia RichText representation. + * + * @param editor - Editor instance + */ + static #setupImageBlockToInlinePostFixer(editor: Editor): void { + const { model } = editor; + const { schema } = model; + + if (!schema.isRegistered("imageBlock")) { + return; + } + + // Allow xlink-href on imageBlock so the upcast can set the attribute + // (preventing GHS from capturing data-xlink-href into htmlImgAttributes). + schema.extend("imageBlock", { + allowAttributes: [ContentImageEditingPlugin.XLINK_HREF_MODEL_ATTRIBUTE_NAME], + }); + + const xlinkHrefAttr = ContentImageEditingPlugin.XLINK_HREF_MODEL_ATTRIBUTE_NAME; + const imageInlineName = ContentImageEditingPlugin.IMAGE_INLINE_MODEL_ELEMENT_NAME; + + model.document.registerPostFixer((writer) => { + let changed = false; + + for (const change of model.document.differ.getChanges()) { + if (change.type === "insert" && change.name === "imageBlock") { + const item = change.position.nodeAfter; + if (!item || !item.is("element", "imageBlock")) { + continue; + } + + const xlinkHref = item.getAttribute(xlinkHrefAttr); + if (!xlinkHref) { + continue; + } + + // Replace the imageBlock with a paragraph containing an imageInline. + const paragraph = writer.createElement("paragraph"); + const imageInline = writer.createElement(imageInlineName, { + [xlinkHrefAttr]: xlinkHref, + }); + writer.append(imageInline, paragraph); + writer.insert(paragraph, item, "before"); + writer.remove(item); + changed = true; + } + + // When a content image is moved (drag-and-drop) out of a paragraph + // that contained only that image, the source paragraph becomes empty. + // CKEditor does not remove it automatically, so we clean it up here. + if (change.type === "remove" && change.name === "imageInline") { + const parent = change.position.parent; + if (parent.is("element", "paragraph") && parent.childCount === 0 && parent.parent !== null) { + writer.remove(parent); + changed = true; + } + } + } + + return changed; + }); + } + /** * Register `imageInline` model elements for subscription cleanup * on model changes. diff --git a/packages/ckeditor5-coremedia-images/src/converters.ts b/packages/ckeditor5-coremedia-images/src/converters.ts index 2eb4e1caa1..55e467ff76 100644 --- a/packages/ckeditor5-coremedia-images/src/converters.ts +++ b/packages/ckeditor5-coremedia-images/src/converters.ts @@ -78,6 +78,77 @@ export const preventUpcastImageSrc = ); }; +/** + * High-priority upcast converter that creates `imageInline` for any + * `` when the current insertion position allows + * inline content (e.g. inside a paragraph). + * + * Without this, `ImageBlockEditing`'s `normal`-priority converter runs first + * and calls `safeInsert` for an `imageBlock`. When the drop cursor is *inside* + * a paragraph (between letters), `safeInsert` splits that paragraph to + * accommodate the block element, leaving empty paragraph fragments behind. + * + * By creating `imageInline` at high priority for inline positions, no + * paragraph splitting occurs. When the cursor is at a block-level position + * (between paragraphs) `imageInline` is not allowed there; the converter + * returns without consuming so that `ImageBlockEditing` can place an + * `imageBlock` at block level. The `imageBlock` post-fixer then converts + * that back to `paragraph + imageInline` without any splitting. + */ +export const upcastContentImageAsInline = + (): ((dispatcher: UpcastDispatcher) => void) => + (dispatcher: UpcastDispatcher): void => { + dispatcher.on( + `element:img`, + (evt: EventInfo, data, conversionApi: UpcastConversionApi) => { + if (!data.viewItem.hasAttribute("data-xlink-href")) { + return; + } + + if (!conversionApi.consumable.test(data.viewItem, { name: true })) { + return; + } + // Only proceed when imageInline is allowed at the current cursor + // position. If not (e.g. between block elements at root level), fall + // through to ImageBlockEditing so the post-fixer can handle it. + + if (!conversionApi.schema.checkChild(data.modelCursor, "imageInline")) { + return; + } + + const xlinkHref = String(data.viewItem.getAttribute("data-xlink-href")); + const imageInline = conversionApi.writer.createElement("imageInline", { + "xlink-href": xlinkHref, + }); + + if (!conversionApi.safeInsert(imageInline, data.modelCursor)) { + return; + } + + // Consume the element name so that ImageBlockEditing and + // ImageInlineEditing (both guard with `consumable.test(viewItem, + // { name: true })`) skip this element — they would otherwise create + // a duplicate model element. + // + // Also consume `data-xlink-href` so the attributeToAttribute upcast + // converter does not try to set xlink-href a second time (we already + // set it directly in createElement above). + // + // IMPORTANT: do NOT call evt.stop() here. Other lower-priority + // element:img listeners include CKEditor's attributeToAttribute + // converters for `alt`, `width`, `height`, etc. They are guarded by + // their own consumable entries ({ attributes: 'alt' }), not by + // { name: true }, so they will still fire, find the imageInline via + // data.modelRange, and set the remaining attributes correctly. + conversionApi.consumable.consume(data.viewItem, { name: true }); + conversionApi.consumable.consume(data.viewItem, { attributes: "data-xlink-href" }); + + conversionApi.updateConversionResult(imageInline, data); + }, + { priority: "high" }, + ); + }; + /** * Conversion for `modelElementName:xlink-href` to `img:src`. *