ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.36k stars 3.68k forks source link

[Windows] Uploaded images disappear when dragged if no alt tag is set in Chromium browsers #15700

Closed rossbearman closed 8 months ago

rossbearman commented 8 months ago

📝 Provide detailed reproduction steps (if any)

  1. Open a CKEditor instance (including the demo builds at ckeditor.com) in a Chromium browser
  2. Upload an image, either by clicking the "Upload image from computer" button, or dragging one onto the editor
  3. Wait for the image to fully upload
  4. Drag the image to a new location without setting the alt tag

✔️ Expected result

The image should reappear in the location it was dragged to, or stay in the same place if the carat hasn't been moved.

❌ Actual result

The image disappears when you release the mouse button after dragging it to move it, even if you only move the mouse a single pixel in any direction. When viewing the source, the element has been removed entirely.

📃 Other details

The issue is reproducible during the session that they are first uploaded and during subsequent sessions.

The behaviour is consistent when adding or removing the alt tag using the source browser. Without an alt tag, or with an empty alt tag (alt=""), the image disappears as soon as it moves, with an alt tag (even containing just a space), the image moves around as you'd expect.

Witoso commented 8 months ago

Hi, thanks for the report! Could you record a video, and explain a bit more this step:

Drag the image to a new location without setting the alt tag

I completed the steps and dragged the image to a different app, worked. Also dragging the picture in the multi-root works (it has multiple different editable).

Thanks!

rossbearman commented 8 months ago

Thanks for the quick reply, I will put a video together now.

Specifically, dragging the uploaded image to anywhere within the same editor, for example dragging it from below a paragraph to above that paragraph. ~However the multi-root editor shows the same behaviour for me when dragging between the different editable areas.~ [ED: This was actually just the image not being uploaded yet on the multi-root editor, however it is reproducible on other builds in the docs, see later posts]

Currently we've tested this on five different Windows machines, two within a corporate network, three personal machines, all exhibit this behaviour in Chrome and Edge.

rossbearman commented 8 months ago

Here's a video demonstrating the behaviour in the demo build. This is recorded in the latest Chrome on Windows 10, the behaviour is consistent in Microsoft Edge, but works correctly in Firefox.

In the video I'm dragging and dropping the image from a local directory, waiting for the uploaded green tick to appear, and then attempting to move it within the editor window. Then doing the same, but first setting the alt tag, which stops the image disappearing, remove the alt tag again causes the image to disappear when moved. At 25 seconds in, I use the "Upload image from computer button" (the video doesn't show the file browser) and get the same outcome. I then use the file manager to insert one of the previously uploaded images, which has the same outcome again when moved.

I was wrong about the multi-root editor in my previous message, it works correctly, along with the "Classic" predefined build in documentation area, however the failure does occur in the Feature-rich Editor in the documentation area. I thought it was also failing in the multi-root demo as it disappears on those if you move it before the image is fully uploaded, but I belive this is the intended behaviour?

The issue is however reproducible in builds under the demos section: ckeditor.com/ckeditor-5/demo, and in the current build used on the latest Drupal release (which I believe is 40.2.0). The source viewer demo shows that the image disappears entirely from the source when it is moved.

rossbearman commented 8 months ago

Here's a follow-up video, demonstrating the same issue in the feature-rich build on the CKEditor docs, both with an uploaded image and with a demo image brought in from CKBox, followed by it working as expected in the multi-root demo.

rossbearman commented 8 months ago

I've gone through and tested every editor under the examples section of the docs. The bug is only present in the custom builds feature-rich build and collaborative editor. All others work as expected.

In the demos section, it fails on the following:

It works correctly on both collaborative editors, user interface configurations, productivity pack, export to PDF/Word, paste from Office, mobile friendly editor and spelling and grammar checker.

EDIT: Have now tested these separately on a different machine in Chrome and Edge on Windows 11, with consistent results.

FilipTokarski commented 8 months ago

I checked it and it looks like the LinkImage plugin is the problem. If I remove this plugin from the editor, then I'm no longer able to reproduce the problem.

Also, I'm able to reproduce it only on Windows, macOS seems to work fine.

@rossbearman could you check on your side if it helps when you remove the LinkImage plugin?

FilipTokarski commented 8 months ago

Also, looks like it was introduced in 40.2.0, as I'm not able to reproduce it on older demos.

rossbearman commented 8 months ago

@rossbearman could you check on your side if it helps when you remove the LinkImage plugin?

Unfortunately not. I disabled the LinkImage plugin through Drupal's CKEditor configuration and while the option to link the images has gone from the image toolbar, the issue has not. The feature rich editor demo also doesn't appear to have LinkImage enabled, but the bug is present there.

Although with LinkImage disabled, when first inserted and saved, the images aren't linked anywhere (either in the interface or the source), but if I move the images, making them disappear, and then Ctrl+Z to get them back, the images are now wrapped in a link to the image itself. When clicked on, the image opens the core edit link dialogue. Removing this link manually doesn't change the behaviour. With LinkImage enabled, the images aren't wrapped in a link even after making them disappear and then bringing them back with Ctrl+Z.

The same behaviour happens in the feature-rich demo and if I then remove the link that it has added, the following error appears in the console:

Uncaught TypeError: Cannot read properties of undefined (reading '_removeAttribute') ck-demo-external-feature-rich-b79de54e60.js:8222

After which the image can no longer be moved:

Uncaught TypeError: Cannot read properties of null (reading 'parent') ck-demo-external-feature-rich-b79de54e60.js:8222

That error only happens on the CKEditor demo, however, and not the Drupal build.

Also, looks like it was introduced in 40.2.0, as I'm not able to reproduce it on older demos.

It looks like between 40.1.0 and 40.2.0 the only changes in the Link plugin in general were to autolink.ts by @TheSpyder and linkediting.ts by @niegowski.

rossbearman commented 8 months ago

I can confirm that disabling the entire Link plugin does resolve the issue in Drupal, although obviously isn't practical in production.

TheSpyder commented 8 months ago

My change (#15134) was designed to be limited to user pasting - but it looks like drag and drop triggers the clipboard pipeline? So that's plausible. It first checks if the entire "pasted" content is a URL, so it should be doing nothing (the drag and drop implementation doesn't appear to even set text/plain on the data transfer).

if I move the images, making them disappear, and then Ctrl+Z to get them back, the images are now wrapped in a link to the image itself

That does sound like my code activated.

Witoso commented 8 months ago

Closed via #15737.