[PP-1] Disable Restrict images for Basic HTML text format because it does not allow remote images

https://github.com/goalgorilla/open_social/pull/549 contains more information:

Try to upload image using Basic HTML text format at any NOT local site

(Emphasis mine.)

This is reducing security of Open Social. It also increases vulnerability to link rot and therefore broken content over a longer period of time.

If I were a maintainer of Open Social, I would not accept this. You'd still be free to change the configuration, but I'd recommend against this.

Yes, #2528214: "Restrict images to this site" blocks image style derivatives is then likely the cause of your problems, and is indeed the proper fix! :)

Will released in version 1.6

@maikelkoopman I think this is a very bad idea. You've just enabled hotlinking of images. Which means:

This is reducing security of Open Social. It also increases vulnerability to link rot and therefore broken content over a longer period of time.

Wim, we have added the patch form the 2528214 https://www.drupal.org/files/issues/image_restrict_image_styles-2528214-.... So the option to disable Restrict images for Basic HTML is not enabled now. We have to go forward with a solution, because it is blocking users from uploading images in the ckeditor.
https://github.com/goalgorilla/open_social/pull/549/files

In your previous comment you said 'and is indeed the proper fix!", I was assuming this is okay to move forward?

The proper fix is to get #2528214: "Restrict images to this site" blocks image style derivatives fixed in core, and in the mean time, the proper solution is to include that patch. You did that in https://www.drupal.org/commitlog/commit/60333/7a7aae1d1db4354b3e4412b3be... (great!), but you also committed https://www.drupal.org/commitlog/commit/60333/109aa3b5a495aede2ef0c9cac7... (bad!).

I think perhaps https://www.drupal.org/commitlog/commit/60333/109aa3b5a495aede2ef0c9cac7... was an accidental commit then? :)

Hi Wim,

The changes in 109aa3b5a495aede2ef0c9cac7b29d8c812b57d1 have been undone in 7a7aae1d1db4354b3e4412b3be5cff5fd48edd0a.

We do not squash the commits when merging pull requests on GitHub, this is probably where the confusion comes from.

The final diff that was committed is https://github.com/goalgorilla/open_social/pull/549.patch. Hope that clears everything up ;)

Eh … ok. This is exactly why you need to squash commits. All good then, but your commit history is now super confusing and pretty useless (since it first makes a change and then undoes it).

It's fine to not squash commits that touch different areas, to have logical commits, but these aren't remotely logical.

Anyway: 👍

I see your point.

@jochemvn mentioned we can start squashing commits when a PR on GitHub is closed.

Anyway, thanks for raising the point! I’ll throw it in the group.

Cool :)