Carousel plugin broken by djangocms-text-ckeditor v3.2.1

[mliudev]

An update to the plugin_tags_to_user_html(...) signature breaks the carousel plugin which calls the method. Commit on djangocms-text-ckeditor which introduces the issue:

https://github.com/divio/djangocms-text-ckeditor/commit/84a6c0aa842a5d71b168bdb70036f6cb546438ae#diff-10487c5f83be0ec14f267e342322a789R109

Fix may be as simple as removing the plugin type from the method call.

[darbula]

Hi @jrief is this issue is the reason for putting warning in README

Currently DjangoCMS-Cascade does not work with djangocms-text-ckeditor >= 3.1. Please stay with version 3.0.1 until this issue hase been fixed.

If it is:

• it might be 3.2.1 instead of 3.1.0 since in 3.1.0 and in 3.2.0 this change is still not introduced
• it would be beneficial to state issue number in README

[jrief]

Never tested with djangocms-text-ckeditor==3.2.1 According to you, does it work with that version?

[darbula]

Me neither I'm still on 3.1.0, but from this report and code changes in djangocms-text-ckeditor it seems that 3.2.1 could be the first version to introduce this problem, and not 3.1.0 as stated in README, maybe @mliudev can confirm this?

[mliudev]

@darbula That's correct, we currently use 3.2.0 which does not have the signature change. 3.2.1 introduces the breaking change.

[pcolmant]

Why not test the ckeditor version ? And pass the correct signature, either plugin_tags_to_user_html(caption, context, placeholder) before 3.2.1 or plugin_tags_to_user_html(caption, context) from 3.2.1 in cmsplugin_cascade/bootstrap3/carousel.py. The only place where it's used in djangocms-cascade.

[jrief]

It's known to work with djangocms-text-ckeditor==3.0.1 – that's the version I support currently. I had no time yet, to check for other versions of ckeditor, but everybody is welcomed to test and fix it.

[pcolmant]

I understand Jacob,

Do you prefer I make a PR ?

To fix it, I checked the call to plugin_tags_to user_html. I only found one call in : cmsplugin_cascade/bootstrap3/carousel.py.

A possible if not better adaptation should be

 def render(self, context, instance, placeholder):
        # image shall be rendered in a responsive context using the ``<picture>`` element
        elements = utils.get_picture_elements(context, instance)
        caption = self.html_parser.unescape(instance.glossary.get('caption', ''))
        fluid = instance.get_complete_glossary().get('fluid') == 'on'
        import djangocms_text_ckeditor
        from distutils.version import LooseVersion
        if(LooseVersion(djangocms_text_ckeditor.__version__) < LooseVersion("3.2.1")):
            caption = plugin_tags_to_user_html(caption, context, placeholder)       
        else:
            caption = plugin_tags_to_user_html(caption, context)
        context.update({
            'is_responsive': True,                  
            'instance': instance,
            'caption': caption,
            'is_fluid': fluid,                 
            'placeholder': placeholder,
            'elements': elements,       
        })          
        return super(CarouselSlidePlugin, self).render(context, instance, placeholder)

I also got another issue and solution with 3 cascades 0.11.0 templates due to USE_L10N = True in my settings : cascade/bootstrap3/gallery.html and image.html and picture.html

I replaced

width="{{ thumb.width }}" height="{{ thumb.height }}"

by

width="{{ thumb.width|stringformat:"s" }}" height="{{ thumb.height|stringformat:"s" }}"

I didn't use the template unlocalize tag because it needed to {% load l10n %}

Without the stringformat, 1440 became 1&nbsp440 which render an hidden img. It's a common issue I encounter with ID and USE_L10N.

By the way, I needed to upgrade to djangocms-text-ckeditor==3.3.1 or the filer plugins didn't work anymore with CMS 3.4.1, like a come back of this old issue : https://github.com/divio/djangocms-text-ckeditor/issues/33. I haven't tested 3.4.0.

The last thing I did is to remove in cascade/admin.change_form.html the h1

{% block field_sets %}
    <h1>{{ plugin_title }}</h1> <--------- remove
    {{ plugin_intro }}

to avoid this

in CKEditor and get this

Thanks for your tools !

Patrick

[jrief]

Yes, if you could create a PR, you would really help me.

One thing I noted is that width is transformed from 1440 into 1&nbsp440 Are you sure it's not 1 440? Maybe try the unlocalize template-filter here.

[pcolmant]

Yes it's with the html non-breaking space, thus with the ;

I will do the PR for end of next week.

[mliudev]

Seems like this is fixed. I haven't tested and I'm not in a position to do so anymore so I will close this issue.