Skip to content

Commit

Permalink
XWIKI-22482: Links wrapping images that have been centered with the o…
Browse files Browse the repository at this point in the history
…ld image dialog are lost on save

* Fix the image widget initialization and downcast when the image was previously inserted with the old image dialog.
  • Loading branch information
mflorea committed Sep 6, 2024
1 parent 41c7ff8 commit 2924e67
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -446,22 +446,39 @@
widget.on('data', function () {
widget.resizer[widget.data.alignment === 'end' ? 'addClass' : 'removeClass']('cke_image_resizer_left');
});

if(!widget.wrapper.getChild(0).hasClass('cke_widget_element')) {
// Re-wrap the element in a widget element.
// This happens when removing the caption of an image.
var widgetElement = editor.document.createElement('span');
widgetElement.addClass('cke_widget_element');
if (resizeWrapper) {
widgetElement.append(resizeWrapper);
}
widget.wrapper.append(widgetElement, true);
widget.element = widgetElement;
widget.element.setAttribute('data-widget', widget.name);
} else {

// The image has been wrapped in the image resize wrapper. If the image was the widget element then we need to
// create another widget element to hold the image resize wrapper (which includes the image).
if (!widget.wrapper.getChild(0).hasClass('cke_widget_element')) {
const oldWidgetElement = widget.element;
const widgetElementAttrs = [
'data-cke-widget-data',
'data-cke-widget-upcasted',
'data-cke-widget-keep-attr',
'data-widget',
];

// Create a new widget element (copying some attributes from the old widget element).
const newWidgetElement = editor.document.createElement('span');
newWidgetElement.addClass('cke_widget_element');
newWidgetElement.setAttributes(widgetElementAttrs.reduce((attrs, name) => {
attrs[name] = oldWidgetElement.getAttribute(name);
return attrs;
}, {}));
if (resizeWrapper) {
widget.element.append(resizeWrapper, true);
newWidgetElement.append(resizeWrapper);
}

// Replace the widget element.
widget.wrapper.append(newWidgetElement, true);
widget.element = newWidgetElement;

// Cleanup the old widget element.
oldWidgetElement.removeClass('cke_widget_element');
oldWidgetElement.removeAttributes(widgetElementAttrs);
} else if (resizeWrapper) {
// Simply append the image resize wrapper to the widget element.
widget.element.append(resizeWrapper, true);
}
}

Expand Down Expand Up @@ -634,49 +651,25 @@
originalData.call(this);
};

var originalUpcast = imageWidget.upcast;
// @param {CKEDITOR.htmlParser.element} element
// @param {Object} data
imageWidget.upcast = function (element, data) {
var el = originalUpcast.apply(this, arguments);
if (el && element.name === 'img') {
// Wrap the image with a span. This span will be used to place the resize span during the init of the image
// widget.
var span = new CKEDITOR.htmlParser.element( 'span' );
el.wrapWith(span);
el = span;
}
return el;
};

var originalDowncast = imageWidget.downcast;
imageWidget.downcast = function (element) {
const originalDowncast = imageWidget.downcast;
imageWidget.downcast = function (...args) {
const alignment = this.data.alignment;
var el = originalDowncast.apply(this, arguments);
downcastLegacyCenter(el, alignment);
var isNotCaptioned = this.parts.caption === null;
if (isNotCaptioned) {
let img;
if(el.name === 'img') {
img = el;
} else {
img = el.findOne('img', true);
const element = originalDowncast.apply(this, args);
downcastLegacyCenter(element, alignment);
if (!this.parts.caption) {
// Image widget without caption.
// Remove the wrapping span used for the resize handle.
const img = element.name === 'img' ? element : element.findOne('img', true);
let imageResizeWrapper = img;
while (imageResizeWrapper && imageResizeWrapper.name !== 'span' && imageResizeWrapper !== element) {
imageResizeWrapper = imageResizeWrapper.parent;
}
// Cleanup and remove the wrapping span used for the resize caret.
delete img.attributes['data-widget'];
if(el.children[0]) {
var firstChild = el.children[0];
if (firstChild.children[0]) {
firstChild.replaceWith(firstChild.children[0]);
}
if (imageResizeWrapper?.name === 'span' && imageResizeWrapper !== element) {
imageResizeWrapper.replaceWith(imageResizeWrapper.children[0]);
}
}

// Safety data-widget removal as I noticed an additional data-widget being persisted. I did not identify the
// exact reproduction steps though.
delete el.attributes['data-widget'];

return el;
return element;
};
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,11 +634,10 @@ void quickInsertImageOtherPage(TestUtils setup,
createAndLoginStandardUser(setup);
// Create a child page.
DocumentReference otherPage = new DocumentReference("attachmentOtherPage",
testReference.getLastSpaceReference());
testReference.getLastSpaceReference());

// Attach an image named "otherImage.gif" to the other page.
String otherAttachmentName = "otherImage.gif";
AttachmentReference attachmentReference = new AttachmentReference(otherAttachmentName, otherPage);
uploadAttachment(setup, otherPage, otherAttachmentName);

String attachmentName = "image.gif";
Expand Down Expand Up @@ -817,6 +816,7 @@ void pasteAndEditExternalImage(TestUtils setup, TestReference testReference) thr
CKEditor editor = new CKEditor("content").waitToLoad();

RichTextAreaElement richTextArea = editor.getRichTextArea();
richTextArea.clear();
richTextArea.sendKeys(Keys.chord(Keys.CONTROL, "v"));
richTextArea.verifyContent(content -> {
content.getImages().get(0).click();
Expand Down Expand Up @@ -864,7 +864,7 @@ void editListWithImage(TestUtils setup, TestReference testReference) throws Exce

@Test
@Order(20)
void editImageWithDataWidgetAttribute(TestUtils setup, TestReference testReference) throws Exception
void editImageWithDataWidgetAttribute(TestUtils setup, TestReference testReference)
{
setup.loginAsSuperAdmin();
ViewPage page = setup.createPage(testReference, "[[image:image.gif||data-widget='uploadimage']]");
Expand All @@ -876,6 +876,32 @@ void editImageWithDataWidgetAttribute(TestUtils setup, TestReference testReferen
assertEquals("[[image:image.gif]]", savedPage.editWiki().getContent());
}

@Test
@Order(21)
void editLegacyCenteredImageWithLink(TestUtils setup, TestReference testReference) throws Exception
{
// Run the tests as a normal user. We make the user advanced only to enable the Edit drop down menu.
createAndLoginStandardUser(setup);

// Upload an attachment to test with.
String attachmentName = "image.gif";
ViewPage newPage = uploadAttachment(setup, testReference, attachmentName);

WikiEditPage wikiEditPage = newPage.editWiki();
wikiEditPage.setContent("(% style='text-align: center' %)\n"
+ "[[~[~[image:image.gif~]~]>>Target.Page]]");
ViewPage viewPage = wikiEditPage.clickSaveAndView();

// Move to the WYSIWYG edition page.
WYSIWYGEditPage wysiwygEditPage = viewPage.editWYSIWYG();
new CKEditor("content").waitToLoad();

ViewPage savedPage = wysiwygEditPage.clickSaveAndView();

assertEquals("[[~[~[image:image.gif~|~|data-xwiki-image-style-alignment=\"center\"~]~]>>Target.Page]]",
savedPage.editWiki().getContent());
}

/**
* Initialize a page with some content and an image. Then, copy its displayed content in the clipboard.
*
Expand Down

0 comments on commit 2924e67

Please sign in to comment.