CKEditor: allow uploading an image.

[quicksketch]

Right now we have the ability to insert an image into a WYSIWYG editor through a modal dialog that is provided by Backdrop. However this modal right now only provides the basics: URL, height/width, and alt attributes. There is very little point in making a dialog that does a server-side request unless we make use of the the server-side abilities that Backdrop provides us. In this case, I'm referring to allowing a user to upload an image in the image modal dialog.

Because the modal dialog is server-side, it means that we can use a #type = 'managed_file' element in the modal to handle file uploads. After the initial problem of uploading a file is handled, we may extend this element (similar to the way FileField Sources does) to provide other ways to select a file, such as using Views to select recent uploads, or to actually transfer images from remote URLs, or to paste from the clipboard, or other ways of managing files.

Drupal issue: https://www.drupal.org/node/1932652

[quicksketch]

I filed a PR at backdrop/backdrop#1015 that adds all this functionality. A lot of the crucial code was already committed a in the first CKEditor PR, but this completes it.

This patch goes a bit further than the original Drupal patch, which only allowed file uploading or URLs, but not both at the same time. Enabling uploading would disable raw URLs, which may be desirable for some sites but probably not the most common use-case.

Screenshot of the upload/URL toggle:

With URL selected:

This needs a few additional tests for the server-side registering of associating content with files based on the data-file-id within the available filtered text areas. Although mostly inline with D8, this differs in the regard that we do not have UUIDs on files and so inline file references are by the FID instead. As no entities (nodes, files, users, etc) have UUIDs at this point, that's a larger issue if we decide to pursue the problem of cross-environment continuous migration.

[docwilmot]

Works great. Few issues:

1. If you upload an image, then insert into editor then doubleclick the image in the textarea, the "Edit image" box pops up as expected. Click "Remove and Save. This leaves a ghost frame of the image in the editor.

the source button reveals this code in the editor: <p><img alt="" data-file-id="8" height="70" src="" width="358" /></p>

1. Similarly if you just click the image button then save, this ends up in the editor: <p><img alt="" src="" /></p>

Granted for both 1 and 2 above, the p tags and content disappear on closing the source view.

1. Related to the editor, and not image uploading per se, when you click the save button, the editor disappears revealing plain HTML while the page reloads.
2. If you add a URL for an image then change your miind and upload an image, but leave the URL in the box, the URL always wins and that is the image you get in your article, even though the uploaded image still reaches the files folder. There should/could be some sort of warning that youve selected to possible sources and the default is URL, maybe.
3. Unfair to mention here but strange image renaming still happens

[quicksketch]

Yeah the handling of blank/removed images is not good. I'll work to make that remove the <img /> tag from the source code if no file is uploaded or in the src field.

Related to the editor, and not image uploading per se, when you click the save button, the editor disappears revealing plain HTML while the page reloads.

Yeah I'm not sure if that problem can be solved (easily). I think that's CKEditor behaviour to remove itself on form submit. A separate issue for us in any case.

If you add a URL for an image then change your miind and upload an image, but leave the URL in the box, the URL always wins and that is the image you get in your article, even though the uploaded image still reaches the files folder. There should/could be some sort of warning that youve selected to possible sources and the default is URL, maybe.

I thought I had covered this situation. When toggling to the upload, the value from the URL should be getting removed via JS (and restored if you toggle back to it). I'll give it another go.

Thanks for testing!

[quicksketch]

Updated the PR to fix these problems. Blanking out an image src string or clicking the "Remove" button now removes the image from the editor.

I couldn't reproduce a problem with using src and an upload at the same time. It seems that using both will use the src string if the "Image URL" is selected, or the uploaded file if the "Image upload" option is selected.

Tests still needed, I'll work on those next.

[docwilmot]

p.s.:

similar to the way FileField Sources does

Already ported! https://github.com/backdrop-contrib/filefield_sources

[quicksketch]

Nice! I'll have to give that a shot in combination with this to see what the overall effect is.

I updated the PR with a second commit that adds some polish. Enabling and disabling buttons now hides/shows related settings. e.g. disabling the "Image" button will hide all image uploading settings. Same for the "Styles" button and associated textarea.

[klonos]

Haven't actually tested this yet, but here's a couple of comments from what I can see from the screenshots:

The textfield for Image URL will allow for both local paths as well as full external URLs (copy-paste image link like say youtube video URLs), right? If so, then shouldn't we call these something like "Upload image" and "Link to image" instead?

The way "Add a caption" works still bothers me (I think I've mentioned this before bt not sure if I filed an issue for it). Why not have a textfield for a caption show directly on the dialog there once the checkbox is ticked?

[docwilmot]

shouldn't we call these something like "Upload image" and "Link to image" instead?

👍

Why not have a textfield for a caption show directly on the dialog there once the checkbox is ticked?

Very yes, but not a 1.2 blocker though.

[klonos]

Turns I have already filed an issue for the caption checkbox thing: point 2 in #1093

[Graham-72]

It is really good to have image upload working - thanks. But I am finding that align center is causing a failure. Perhaps this is just my trial installation? What should it be achieving in the html?

[quicksketch]

But I am finding that align center is causing a failure. Perhaps this is just my trial installation? What should it be achieving in the html?

In the HTML it should be creating a <img src="foo" data-align="center" /> If you have a caption it should also add data-caption="The caption.". Under what circumstances does it not seem to be working?

Tests have been in place for a week now and it sounds like the testing on this functionality has been positive. Unless there's anything around this issue, I think we may be ready for the commit on this functionality.

[Graham-72]

I uploaded from core in backdrop-1088-editor-upload to replace core in a previous version and everything seems to work fine, including the align left and align radio buttons, but align center causes a freeze, the image disappears and I am unable to view the source code.
The most likely explanation is that I have missed something in my upload.

[quicksketch]

Great, thanks for the report Graham. I bet there's a JS error on the page. I'll see if I can reproduce.

[quicksketch]

but align center causes a freeze, the image disappears and I am unable to view the source code.

I've fixed this issue in the latest changes to the PR. backdrop/backdrop#1015

There were some other issues relating to removing an image's src within the dialog while a caption was present. The whole thing wasn't getting cleaned up properly.

[Graham-72]

It works fine now. Great job 👍

[quicksketch]

Super, let's merge and improve further if needed.

[quicksketch]

Merged in backdrop/backdrop#1015. Exciting times!

[Graham-72]

Oh dear! Tried editing a block but could not upload an image - no response from 'save'.

Also, suppose I want to insert an image that I have already uploaded? Please can we have a browse server without upload, if you get what I mean?

[quicksketch]

Tried editing a block but could not upload an image - no response from 'save'.

I'll look into that problem here.

Also, suppose I want to insert an image that I have already uploaded? Please can we have a browse server without upload, if you get what I mean?

Yeah, that would be useful functionality. Short-term, we could make FileField Sources do this (as it just extends the "managed_file" element, which this uses). Long-term, I think we'll want to make a more comprehensive image/media dialog similar to what WordPress has. Reselecting an existing file isn't a trivial problem, as the way you would want to select an image (e.g. recently uploaded by user, recently uploaded by field, tags, filenames, etc.) can be challenging.

[MrHaroldA]

as the way you would want to select an image can be challenging.

This should be a view ...

[Graham-72]

I have just been talking a new user though the process of adding an image to a page and notice problems with resizing by changing the width and height properties* compared to using IMCE and the CKEditor config module, (a) the width and height do not now appear until after saving the upload, and (b) there is no option to lock the ratio of width to height.

*perhaps this is a lazy practice, but it can be very useful

[quicksketch]

(a) the width and height do not now appear until after saving the upload, and (b) there is no option to lock the ratio of width to height.

Yep, this would also be a great thing to add. For now, if exact heights are necessary, you can resize the image with the drag handle in the editor.

This should be a view ...

Yeah agreed.

[Graham-72]

you can resize the image with the drag handle in the editor

I hadn't realised this - it is a great feature - thanks.

[Graham-72]

My previous comment

Tried editing a block but could not upload an image - no response from 'save'.

I have been checking again and this only happens if editing a layout, then configuring a block and trying to edit the content when the edit window appears.

So not really a significant problem because rather an exceptional situation.

[quicksketch]

I have been checking again and this only happens if editing a layout, then configuring a block and trying to edit the content when the edit window appears.

Ah, yes confirmed. It looks like this is an issue because the image dialog closes the layout block-edit dialog. Then when the save button is clicked on the image dialog, the CKEditor instance is no longer on the page. Nested dialogs are tricky business! We may be able to fix this by having the image dialog open in a modal with a different ID.

[quicksketch]

I have been checking again and this only happens if editing a layout, then configuring a block and trying to edit the content when the edit window appears.

I added a new issue for this at #1125 and added to the meta.

I also added a few placeholders in the meta. If we come up with more ideas to expand editor functionality, let's add them there: #1087