From 2e79df1681b805fbb72c490b56c94615a5f508b3 Mon Sep 17 00:00:00 2001
From: Himanshu Kapoor <info@fleon.org>
Date: Wed, 18 Dec 2024 10:01:20 +0000
Subject: [PATCH] Fix issue in RTE with image resize
If you upload an image in PTE and resize it in RTE, the image
was lost. This commit fixes it.
- Introduce getSourceMapAttributes function for centralized attribute management
- Update image and playable extensions to use new source map attributes
- Refactor sourcemap extension to use centralized attribute function
- Modify image serialization logic to handle different source tag names
- Add drag functionality for element manipulation in tests
- Implement image resizing test in content editor
Changelog: fixed
---
.../content_editor/extensions/image.js | 2 ++
.../content_editor/extensions/playable.js | 2 ++
.../content_editor/extensions/sourcemap.js | 29 ++--------------
.../services/markdown_sourcemap.js | 33 +++++++++++++++++++
.../services/serializer/image.js | 2 +-
.../services/serializer/image_spec.js | 14 ++++++++
.../helpers/rich_text_editor_helpers.rb | 18 ++++++++++
.../rich_text_editor/media_shared_examples.rb | 21 ++++++++++++
8 files changed, 93 insertions(+), 28 deletions(-)
diff --git a/app/assets/javascripts/content_editor/extensions/image.js b/app/assets/javascripts/content_editor/extensions/image.js
index 465f75624ad44..f7deabb538504 100644
--- a/app/assets/javascripts/content_editor/extensions/image.js
+++ b/app/assets/javascripts/content_editor/extensions/image.js
@@ -2,6 +2,7 @@ import { Image } from '@tiptap/extension-image';
import { VueNodeViewRenderer } from '@tiptap/vue-2';
import { PARSE_HTML_PRIORITY_HIGH } from '../constants';
import ImageWrapper from '../components/wrappers/image.vue';
+import { getSourceMapAttributes } from '../services/markdown_sourcemap';
const resolveImageEl = (element) =>
element.nodeName === 'IMG' ? element : element.querySelector('img');
@@ -76,6 +77,7 @@ export default Image.extend({
default: false,
renderHTML: () => '',
},
+ ...getSourceMapAttributes(resolveImageEl),
};
},
parseHTML() {
diff --git a/app/assets/javascripts/content_editor/extensions/playable.js b/app/assets/javascripts/content_editor/extensions/playable.js
index 3ac44c28f7e92..24c2d6e64ee48 100644
--- a/app/assets/javascripts/content_editor/extensions/playable.js
+++ b/app/assets/javascripts/content_editor/extensions/playable.js
@@ -1,6 +1,7 @@
import { Node } from '@tiptap/core';
import { VueNodeViewRenderer } from '@tiptap/vue-2';
import PlayableWrapper from '../components/wrappers/playable.vue';
+import { getSourceMapAttributes } from '../services/markdown_sourcemap';
const queryPlayableElement = (element, mediaType) => element.querySelector(mediaType);
@@ -37,6 +38,7 @@ export default Node.create({
parseHTML: (element) =>
queryPlayableElement(element, this.options.mediaType).getAttribute('height'),
},
+ ...getSourceMapAttributes((element) => queryPlayableElement(element, this.options.mediaType)),
};
},
diff --git a/app/assets/javascripts/content_editor/extensions/sourcemap.js b/app/assets/javascripts/content_editor/extensions/sourcemap.js
index 8ac4a7d07200f..1eb0efb2b0dd2 100644
--- a/app/assets/javascripts/content_editor/extensions/sourcemap.js
+++ b/app/assets/javascripts/content_editor/extensions/sourcemap.js
@@ -1,5 +1,5 @@
import { Extension } from '@tiptap/core';
-import { getMarkdownSource, docHasSourceMap } from '../services/markdown_sourcemap';
+import { getSourceMapAttributes } from '../services/markdown_sourcemap';
import Audio from './audio';
import Blockquote from './blockquote';
import Bold from './bold';
@@ -37,8 +37,6 @@ export default Extension.create({
name: 'sourcemap',
addGlobalAttributes() {
- const preserveMarkdown = () => gon.features?.preserveMarkdown;
-
return [
{
types: [
@@ -75,30 +73,7 @@ export default Extension.create({
Video.name,
...HTMLNodes.map((htmlNode) => htmlNode.name),
],
- attributes: {
- /**
- * The reason to add a function that returns an empty
- * string in these attributes is indicate that these
- * attributes shouldn’t be rendered in the ProseMirror
- * view.
- */
- sourceMarkdown: {
- default: null,
- parseHTML: (element) => (preserveMarkdown() ? getMarkdownSource(element) : null),
- renderHTML: () => '',
- },
- sourceMapKey: {
- default: null,
- parseHTML: (element) => (preserveMarkdown() ? element.dataset.sourcepos : null),
- renderHTML: () => '',
- },
- sourceTagName: {
- default: null,
- parseHTML: (element) =>
- preserveMarkdown() && docHasSourceMap(element) ? element.tagName.toLowerCase() : null,
- renderHTML: () => '',
- },
- },
+ attributes: getSourceMapAttributes(),
},
];
},
diff --git a/app/assets/javascripts/content_editor/services/markdown_sourcemap.js b/app/assets/javascripts/content_editor/services/markdown_sourcemap.js
index 969130795dd26..0b2966f9b0a31 100644
--- a/app/assets/javascripts/content_editor/services/markdown_sourcemap.js
+++ b/app/assets/javascripts/content_editor/services/markdown_sourcemap.js
@@ -1,3 +1,7 @@
+import { identity } from 'lodash';
+
+const preserveMarkdown = () => gon.features?.preserveMarkdown;
+
export const docHasSourceMap = (element) => {
const commentNode = element.ownerDocument.body.lastChild;
return Boolean(commentNode?.nodeName === '#comment' && commentNode.textContent);
@@ -24,6 +28,15 @@ const getRangeFromSourcePos = (sourcePos) => {
};
};
+const getMarkdownSourceKey = (element) => {
+ return element.dataset.sourcepos;
+};
+
+const getSourceTagName = (element) => {
+ if (!docHasSourceMap(element)) return undefined;
+ return element.tagName.toLowerCase();
+};
+
export const getMarkdownSource = (element) => {
if (!element.dataset.sourcepos) return undefined;
@@ -59,3 +72,23 @@ export const getMarkdownSource = (element) => {
return undefined;
}
};
+
+export const getSourceMapAttributes = (queryElement = identity) => {
+ return {
+ sourceMarkdown: {
+ default: null,
+ parseHTML: (el) => (preserveMarkdown() ? getMarkdownSource(queryElement(el)) : null),
+ renderHTML: () => '',
+ },
+ sourceMapKey: {
+ default: null,
+ parseHTML: (el) => (preserveMarkdown() ? getMarkdownSourceKey(queryElement(el)) : null),
+ renderHTML: () => '',
+ },
+ sourceTagName: {
+ default: null,
+ parseHTML: (el) => (preserveMarkdown() ? getSourceTagName(queryElement(el)) : null),
+ renderHTML: () => '',
+ },
+ };
+};
diff --git a/app/assets/javascripts/content_editor/services/serializer/image.js b/app/assets/javascripts/content_editor/services/serializer/image.js
index de809d0bf2e3b..e0b5deab38b40 100644
--- a/app/assets/javascripts/content_editor/services/serializer/image.js
+++ b/app/assets/javascripts/content_editor/services/serializer/image.js
@@ -17,7 +17,7 @@ const image = preserveUnchanged({
if (realSrc.startsWith('data:') || realSrc.startsWith('blob:')) return;
if (realSrc) {
- if (sourceTagName && !sourceMarkdown) {
+ if (sourceTagName === 'img' && !sourceMarkdown) {
const attrs = pickBy({ alt, title, width, height }, identity);
state.write(openTag(sourceTagName, { src: realSrc, ...attrs }));
return;
diff --git a/spec/frontend/content_editor/services/serializer/image_spec.js b/spec/frontend/content_editor/services/serializer/image_spec.js
index d20f32717ed0b..32310ff265d6c 100644
--- a/spec/frontend/content_editor/services/serializer/image_spec.js
+++ b/spec/frontend/content_editor/services/serializer/image_spec.js
@@ -111,3 +111,17 @@ it('serializes image as an HTML tag if sourceTagName is defined', () => {
'<img src="img.jpg" alt="image" title="image title">',
);
});
+
+it('does not serialize image as HTML if sourceTagName is not img', () => {
+ const imageAttrs = { src: 'img.jpg', alt: 'image', ...sourceTag('a') };
+
+ expect(serialize(paragraph(image(imageAttrs)))).toBe('');
+
+ expect(serialize(paragraph(image({ ...imageAttrs, width: 300, height: 300 })))).toBe(
+ '{width=300 height=300}',
+ );
+
+ expect(serialize(paragraph(image({ ...imageAttrs, title: 'image title' })))).toBe(
+ '',
+ );
+});
diff --git a/spec/support/helpers/rich_text_editor_helpers.rb b/spec/support/helpers/rich_text_editor_helpers.rb
index 22cf169f6e099..8ebade8b1c276 100644
--- a/spec/support/helpers/rich_text_editor_helpers.rb
+++ b/spec/support/helpers/rich_text_editor_helpers.rb
@@ -57,4 +57,22 @@ def click_edit_diagram_button
def expect_drawio_editor_is_opened
expect(page).to have_css('#drawio-frame', visible: :hidden)
end
+
+ def drag_element(element, dx, dy)
+ page.execute_script(<<-JS, element, dx, dy)
+ function simulateDragDrop(element, dx, dy) {
+ const rect = element.getBoundingClientRect();
+ const events = ['mousedown', 'mousemove', 'mouseup'];
+ events.forEach((eventType, index) => {
+ const event = new MouseEvent(eventType, {
+ bubbles: true,
+ screenX: rect.left + (index ? dx : 0),
+ screenY: rect.top + (index ? dy : 0)
+ });
+ element.dispatchEvent(event);
+ });
+ }
+ simulateDragDrop(arguments[0], arguments[1], arguments[2]);
+ JS
+ end
end
diff --git a/spec/support/shared_examples/features/rich_text_editor/media_shared_examples.rb b/spec/support/shared_examples/features/rich_text_editor/media_shared_examples.rb
index 9e5976d126f94..3d51eb53dc1f3 100644
--- a/spec/support/shared_examples/features/rich_text_editor/media_shared_examples.rb
+++ b/spec/support/shared_examples/features/rich_text_editor/media_shared_examples.rb
@@ -24,4 +24,25 @@
expect_media_bubble_menu_to_be_visible
end
end
+
+ describe 'resizing images' do
+ it 'renders correctly with an image as initial content after image is resized' do
+ click_attachment_button
+
+ switch_to_content_editor
+ display_media_bubble_menu '[data-testid="content_editor_editablebox"] img[src]', 'dk.png'
+
+ within content_editor_testid do
+ drag_element(find('[data-testid="image-resize-se"]'), -200, -200)
+ end
+
+ wait_until_hidden_field_is_updated(/width=/)
+ switch_to_markdown_editor
+
+ textarea_value = page.find('textarea').value
+
+ expect(textarea_value).to start_with('
+ expect(textarea_value).to end_with('/dk.png){width=260 height=182}')
+ end
+ end
end
--
GitLab