netresearch / t3x-rte_ckeditor_image

Image support in CKEditor for the TYPO3 ecosystem
GNU Affero General Public License v3.0
56 stars 65 forks source link

[BUGFIX] #122: Fix jquery requirement to not crash the ckeditor #127

Closed mcmulman closed 3 years ago

mcmulman commented 3 years ago

jQuery wasn't always available when running image script. This crashed the whole editor functionality.

Fixes #122

sypets commented 3 years ago

I can confirm that this fixes the problem described in #122. I had the same issue on TYPO3 10.4.18 with rte_ckeditor_image 10.1.1 and can now no longer reproduce the problem with this PR.

However, to get patch to work, I had to flush browser caches. Just flushing TYPO3 caches (page, system) and doing a hard reload in the backend was not enough. I don't know how the caching of assets works in the BE - it may also be enough to increase the version of the extension.

sypets commented 3 years ago

It looks like more is being changed with this patch. I would focus on fixing the problem only first. That makes it easier to review.

Also, some diffs are not real diffs but are due to different indenting. This makes it really cumbersome to review. If common CGL is applied (e.g. by introducing .editorconfig and formatting checks in separate patch) this might be a good idea.

see also https://github.com/TYPO3-Documentation/tea which is the current work in progress best practice example for extensions.

Apart from that PR seems to do the job.

Just my 2c, I would really like to have the problem fixed, because this is quite a nuisance in daily work. Using locally patched version now.