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

Added disableImageUpload to imagePlugin #537

Closed
wants to merge 2 commits into from
Closed

Conversation

rissois
Copy link
Contributor

@rissois rissois commented Jul 14, 2024

Configuration flag for imagePlugin to disable image upload. Ideal for projects that are not self-hosting images.

NOTE: One further improvement to this PR would be to eliminate the dropEffect when an outside image is brought onto the page.

@petyosi
Copy link
Contributor

petyosi commented Jul 15, 2024

This makes sense, I will review and merge in a few days

@rissois
Copy link
Contributor Author

rissois commented Jul 15, 2024

@petyosi

Sorry, I rolled back on the onDrop changes. First, just to confirm the different interactions:

  1. Image dialog upload > call imageUploadHandler > link to external URL
  2. Image dialog URL > link to external URL
  3. Copy and paste
  • from within editor > link to external URL
  • from other website > link to external URL
  • from computer file > call imageUploadHandler > link to external URL
  1. Drag and drop
  • from within editor > link to external URL
  • from other website > link to external URL
  • from computer file > call imageUploadHandler > link to external URL

I think the easiest solution is to check the src for /^https?:\/\// prior to imageeUploadHandler in order to block all local, non-HTTP(S) images. Also want to remove the dropEffect/effectAllowed. Unfortunately I did not know how to make those changes in the right location to block both Copy and paste + Drag and drop. Another major issue I was running into in development is that drag-and-drop in the Ladle example produces an element instead of a markdown . Therefore, I'm not really sure I am getting the true behavior.

Sorry I was not able to complete the PR on my own.

@petyosi
Copy link
Contributor

petyosi commented Jul 20, 2024

@rissois Thank you - your idea is right, but I don't think we need an additional parameter. Not having an upload handler should be enough to disable the upload button and the D & D from the drag start.

I've published a fix for this in v3.9.1. Let me know if this resolves the problem for you.

@petyosi petyosi closed this Jul 20, 2024
@rissois
Copy link
Contributor Author

rissois commented Jul 22, 2024

@petyosi Thanks! That makes a lot of sense, I just wasn't sure the implications of them being mutually exclusive.

Unfortunately, the fix seems to have created a few bugs:

  1. Can no longer use ImageDialog to add a link. Possible solution: add a hidden input field to still register the file (if that is the true root of the problem)
{imageUploadHandler === null ? (
  <input type="hidden" accept="image/*" {...register('file')} />
) : (
  <div className={styles.formField}>
    <label htmlFor="file">{t('uploadImage.uploadInstructions', 'Upload an image from your device:')}</label>
    <input type="file" accept="image/*" {...register('file')} />
  </div>
)}
  1. Copy and paste from another website creates <img height="0" width="0" title="filename.jpg" src="https://...." /> instead of !()[https://...]. The lack of dimensions effective hides the image (this one stumped me)
  2. Unable to drag and drop !()[https://...] images. Possible solution: check if src is originating from outside of a website
if (!imageUploadHandler) {
  const isExternalImage = /^https?:\/\//.test(node.__src)
    if (isExternalImage) {
      return false
    }
}
  1. ImageWithNoBackend example was still given imageUploadHandler. Just need to delete that one line.

@petyosi
Copy link
Contributor

petyosi commented Jul 23, 2024

@rissois thank you - some of those things should be a matter of configuration that's not related to the upload handler - to be more specific, some sort of a validation of which images can be accepted.

I will review what you've described at some point, but things are fairly busy on my side, so if you have the capacity, I'm happy to accept a PR on the matters.

@rissois
Copy link
Contributor Author

rissois commented Jul 23, 2024

@petyosi Happy to help, I managed to figure out how to separate the image types in a method more consistent with your fix. Please see PR #549

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

Successfully merging this pull request may close these issues.

2 participants