-
Notifications
You must be signed in to change notification settings - Fork 337
fix(composer): keep resized image dimensions in sent messages #13482
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
Open
kesselb
wants to merge
1
commit into
main
Choose a base branch
from
ckeditor-keep-resize
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| /** | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| import type { ViewDocumentFragment, ViewElement } from 'ckeditor5' | ||
|
|
||
| import { ImageUtils, Plugin, UpcastWriter } from 'ckeditor5' | ||
|
|
||
| /** | ||
| * Parse a CSS length into whole pixels. Anything but an absolute pixel value | ||
| * yields null. | ||
| * | ||
| * @param value the CSS length to parse | ||
| */ | ||
| function toPixels(value: string | undefined): number | null { | ||
| if (value === undefined) { | ||
| return null | ||
| } | ||
|
|
||
| const pixels = /^\s*([\d.]+)\s*px\s*$/.exec(value) | ||
| if (pixels === null) { | ||
| return null | ||
| } | ||
|
|
||
| const width = Math.round(Number.parseFloat(pixels[1])) | ||
|
|
||
| return width > 0 ? width : null | ||
| } | ||
|
|
||
| /** | ||
| * Sizes images in the editor's data output only; the editing view keeps | ||
| * CKEditor's own markup. | ||
| */ | ||
| export default class ImageDowncastPlugin extends Plugin { | ||
| static get requires() { | ||
| return [ImageUtils] as const | ||
| } | ||
|
|
||
| static get pluginName() { | ||
| return 'ImageDowncast' as const | ||
| } | ||
|
|
||
| init(): void { | ||
| // The width and the natural size are written by two separate converters, | ||
| // so post-process the finished view instead of overriding either. | ||
| this.editor.data.on('toView', (event) => { | ||
| const fragment = event.return as ViewDocumentFragment | ||
| const writer = new UpcastWriter(fragment.document) | ||
|
|
||
| for (const { item } of writer.createRangeIn(fragment)) { | ||
| // A block image carries the resized width on its figure, an inline | ||
| // one on the img itself. | ||
| if ((item.is('element', 'figure') && item.hasClass('image')) || item.is('element', 'img')) { | ||
| this._mirrorResizedWidth(writer, item) | ||
| } | ||
| } | ||
| }, { priority: 'low' }) | ||
| } | ||
|
|
||
| /** | ||
| * Mirrors a resized image's CSS width onto the img width attribute, which | ||
| * clients that drop CSS still honour. Reopening the message reads that width | ||
| * back as the image's natural size. | ||
| * | ||
| * @param writer view writer of the data view | ||
| * @param element the figure or img holding the resized width | ||
| */ | ||
| _mirrorResizedWidth(writer: UpcastWriter, element: ViewElement): void { | ||
| const resizedWidth = toPixels(element.getStyle('width')) | ||
| if (resizedWidth === null) { | ||
| return | ||
| } | ||
|
|
||
| const image = this.editor.plugins.get('ImageUtils').findViewImgElement(element) | ||
| if (image === undefined) { | ||
| return | ||
| } | ||
|
|
||
| writer.setAttribute('width', String(resizedWidth), image) | ||
| // A natural height next to the smaller width would stretch the image. | ||
| writer.removeAttribute('height', image) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
112 changes: 112 additions & 0 deletions
112
src/tests/unit/ckeditor/image/ImageDowncastPlugin.spec.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| /** | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| import { ClassicEditor, ImageBlock, ImageInline, ImageResizeEditing, Paragraph } from 'ckeditor5' | ||
| import ImageDowncastPlugin from '../../../../ckeditor/image/ImageDowncastPlugin.ts' | ||
|
|
||
| // The editor UI observes the size of its toolbar, which jsdom does not provide. | ||
| window.ResizeObserver = class { | ||
| observe() {} | ||
| unobserve() {} | ||
| disconnect() {} | ||
| } | ||
|
|
||
| /** | ||
| * Get the data of an editor initialised with the given content. | ||
| * | ||
| * @param {string} initialData content to load into the editor | ||
| * @return {Promise<string>} the editor's data output | ||
| */ | ||
| async function downcast(initialData) { | ||
| const element = document.createElement('div') | ||
| document.body.appendChild(element) | ||
|
|
||
| const editor = await ClassicEditor.create(element, { | ||
| licenseKey: 'GPL', | ||
| initialData, | ||
| plugins: [Paragraph, ImageBlock, ImageInline, ImageResizeEditing, ImageDowncastPlugin], | ||
| image: { | ||
| resizeUnit: 'px', | ||
| }, | ||
| }) | ||
|
|
||
| const data = editor.data.get() | ||
| await editor.destroy() | ||
| element.remove() | ||
|
|
||
| return data | ||
| } | ||
|
|
||
| /** | ||
| * A 400x300 image resized to the given CSS width. | ||
| * | ||
| * @param {string} width the CSS width on the figure | ||
| * @return {string} the figure markup | ||
| */ | ||
| function resized(width) { | ||
| return `<figure class="image image_resized" style="width:${width};">` | ||
| + '<img src="test.png" width="400" height="300">' | ||
| + '</figure>' | ||
| } | ||
|
|
||
| describe('ImageDowncastPlugin', () => { | ||
| it('mirrors a resized width onto the width attribute', async () => { | ||
| const data = await downcast(resized('200px')) | ||
|
|
||
| expect(data).toContain('width="200"') | ||
| expect(data).toContain('width:200px;') | ||
| }) | ||
|
|
||
| it('drops the natural height so the image is not stretched', async () => { | ||
| const data = await downcast(resized('200px')) | ||
|
|
||
| expect(data).not.toContain('height=') | ||
| }) | ||
|
|
||
| it('keeps the aspect ratio the editor writes', async () => { | ||
| const data = await downcast(resized('200px')) | ||
|
|
||
| expect(data).toContain('aspect-ratio:400/300;') | ||
| }) | ||
|
|
||
| it('keeps the attributes of an image resized to a percentage', async () => { | ||
| const data = await downcast(resized('50%')) | ||
|
|
||
| expect(data).toContain('width="400"') | ||
| expect(data).toContain('height="300"') | ||
| }) | ||
|
|
||
| it('keeps the attributes of an image that was not resized', async () => { | ||
| const data = await downcast('<figure class="image"><img src="test.png" width="400" height="300"></figure>') | ||
|
|
||
| expect(data).toContain('width="400"') | ||
| expect(data).toContain('height="300"') | ||
| }) | ||
|
|
||
| it('mirrors the width of a linked image', async () => { | ||
| const data = await downcast('<figure class="image image_resized" style="width:100px;">' | ||
| + '<a href="https://nextcloud.com"><img src="test.png" width="400" height="300"></a>' | ||
| + '</figure>') | ||
|
|
||
| expect(data).toContain('width="100"') | ||
| expect(data).not.toContain('height=') | ||
| }) | ||
|
|
||
| it('mirrors the width of a resized inline image', async () => { | ||
| const data = await downcast('<p>text <img class="image_resized" style="width:200px;" src="test.png" width="400" height="300"></p>') | ||
|
|
||
| expect(data).toContain('width="200"') | ||
| expect(data).not.toContain('height=') | ||
| }) | ||
|
|
||
| it('keeps the width when the message is reopened and sent again', async () => { | ||
| const sent = await downcast(resized('200px')) | ||
| const resent = await downcast(sent) | ||
|
|
||
| expect(resent).toContain('width="200"') | ||
| expect(resent).toContain('width:200px;') | ||
| expect(resent).not.toContain('height=') | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the required SPDX block-comment format in both new files. Each header starts with
/**, but the required format starts with/*.src/ckeditor/image/ImageDowncastPlugin.ts#L1-L4: change the opening delimiter from/**to/*.src/tests/unit/ckeditor/image/ImageDowncastPlugin.spec.js#L1-L4: change the opening delimiter from/**to/*.As per coding guidelines, “Header format: /* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */”.
📍 Affects 2 files
src/ckeditor/image/ImageDowncastPlugin.ts#L1-L4(this comment)src/tests/unit/ckeditor/image/ImageDowncastPlugin.spec.js#L1-L4Source: Coding guidelines