Skip to content
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

An image with both a caption and a link will apply the link to the caption #69

Open
quicksketch opened this issue Oct 25, 2023 · 5 comments

Comments

@quicksketch
Copy link
Member

quicksketch commented Oct 25, 2023

When both a caption and link are used on a single image, the link is applied to the caption content.

Steps to reproduce:

  • Create a new node with CKEditor 5 as the editor.
  • Insert an image.
  • Enable the caption and enter caption text.
  • Link the image to /home.
  • Save the node.
  • On view, everything works correctly.
  • Edit the node.
  • Upon CKEditor loading, the caption is now linked to /home, separate from the image, which is also linked to /home.

There are some other funky situations:

  • If the caption is already a link, then it is left alone.
  • If part of the caption is already a link, then that link is left intact, but the rest of the caption gets wrapped in a link that duplicates the image link.

As far as I can tell, this is not an issue with our upcast code in viewImageToModelImage(), which seems to have everything correct. But a later converter is causing the caption to be wrapped in the image's link during upcast.

Originally posted by @quicksketch in #54 (comment)

This corresponds to Drupal issue https://www.drupal.org/project/drupal/issues/3396483

@indigoxela
Copy link
Member

Moved my comment from wrong issue here. (Too many opened issue tabs in my browser)

It's caused by CKEditor. I verified with a vanilla setup (Classic editor), without Backdrop involved, only with bare HTML and CKE v 40.0.0 from cdn. But that doesn't seem to be a new problem, possibly even "expected behavior" which should be caught by the LinkImage plugin ... but somehow...

@quicksketch quicksketch added the upstream May be fixed by CKEditor 5 core label Oct 27, 2023
@indigoxela
Copy link
Member

BTW, this faulty behavior also triggers the "reformat" warning when opening the node form. In this case it's actually sort of justified. 😉

@indigoxela
Copy link
Member

indigoxela commented Nov 25, 2023

Another funny way to trigger the problem: drag and drop the image inside the editor. Just a tiny bit, the image doesn't even have to move. As soon as the image gets dropped, the caption gets the link. Hey, is the upcast code even involved in drag/drop?

(Off topic: linked images in (floated) figure with figcaption are the final boss for quite every WYSIWYG editor 👹)

Edit: the official demo does not show this problem, although also using v 40.1.0 so I removed the "upstream" label now.
It's something specific to our (and Drupal 11?) code.

@herbdool
Copy link
Contributor

This is probably far-fetched but I just came across "stretched links" in Bootstrap 5 https://getbootstrap.com/docs/5.3/helpers/stretched-link/ and this problem made me think of that. Perhaps don't wrap the image at all in a link and instead use css to make the image clickable. I'm sure there are lots of issues with my suggestion.

@indigoxela indigoxela removed the upstream May be fixed by CKEditor 5 core label Nov 26, 2023
@indigoxela
Copy link
Member

Not sure, if this issue is a blocker to move this module into core.

Currently lots of things work, only linking images with captions is a mess.
It appears to be a very tricky bug, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants