List of TinyMCE defects in Plone 5.2 / call for fixing issues and participation

I think it is wrong to strip tags, better to give a good user experience and implement browser standards for security, instead of implementing stripping of style you can add on dom with javascript... quite useless!

To answer, if you need style, use the jquery .css() on the tag to set it.

Bear with me if I remember wrong, but the idea was to do all the filtering one place, with https://lxml.de/api/lxml.html.clean.Cleaner-class.html

Having all these things not working is just 'crazy'.

It looks like a lot of the bugs are because of 'style' being stripped.
Disabling filtering is a bad idea (someone will paste some javascript).

I did the following:

  1. Remove style from 'dangerous codes'
  2. Add it 'in the other two' (see screenshot)
  3. Disable html filtering
  4. Save
  5. Revert 'disable filtering'
  6. Save

Style now seems to 'work', … for now.

If it should be of any interest… these are what it is like (now)

1 Like

This is how strikethrough and indent/outdent work. Possibly others, but I've only investigated these. I don't follow your subsequent questions.

There is is pull request on mockup from march 2019 to upgrade to TinyMCE 5.0.1 which seems not that invasive/large. Biggest issue wille be compatibility testing.

But the currently used TinyMCE 4.x hasn't been updated in mockup for a while either, we are using 4.7.13 from the https://github.com/artursmirnov/tinymce_builded repo, where tinymce itself is now at 4.9.10. @frapell and @erral have been doing most of the effort by updating the builded versions of tinymce in that repo.

I do realise a lot of the issues above are around html filtering, which updating TinyMCE itself might not solve. But it could save us from hunting down other obscure TinyMCE issues that have been fixed upstream if we stick closer to their releases/updates.

Disable html filtering
Save
Revert 'disable filtering'
Save

I remember having similar issues where the html filtering worked half ass-ed and this kind of trick worked. @mauritsvanrees looked at it and solved part of the issue, but I cannot find back the issue/ticket on github. :frowning:

Some time ago I updated TinyMCE in the tinymce_bundled repository to have the latests translation files, but I haven't done more on that.

I think that Plone was using the tinymce_bundled because back then it was available for bower and then we just kept using it. Some time ago the author said that he was away from tinymce (see https://github.com/artursmirnov/tinymce_builded/pull/17#issuecomment-421381318) and then I think that only @frapell has made some more improvements.

Perhaps it's the time to go away from that tinymce package and go to the official tinymce package in npm? https://www.npmjs.com/package/tinymce

All 'those' are related to 'style'.
Justify left, right, center
Width of table
Color of text (etc) if you enable that.

horizontal line is a separate tag which can be enabled (not sure if it is hr or hr/ , but adding it 'works'.

Maybe even more confusing (?) is that you can set some properties for table from the format menu

Those serve different purposes. The formats pullout is the set of table classes you can define in the control panel. It makes sense to me to have these Headers/Block/Tables/etc sections grouped together for defining the class(es) that these belong to. The table dropdown is properties specific to a table.

I am still in favor of @zopyx's point though. You can configure the entire toolbar in Plone already and my company does this to be less noisy, but it would be good to clean up a bit for Plone core.

Besides the usability discussion: all the BUG's menitoned by @zopyx above are gone if you

  1. go to /@@filter-controlpanel
  2. add hr to "allowed tags"
  3. add style to "custom attributes"

I've just tried this on demo.plone.org/de and it just works.
So why not simply do this per default in Products.CMPlone.interfaces.controlpanel.IFilterSchema ?

Sometimes solutions can be simple...

+1

PR's are done:


though plone.app.upgrade needs according upgradesteps for existing installations ...

2 Likes

ugprade steps are here

HTMLFilter PRs and upgradesteps are merged ... should be fixed in 5.1.7/5.2.2 ! @mauritsvanrees when are the next releases planned?

@petschki as announced by Maurits in the topic Policy: Plone supported versions , He is unavailable until (early?) June. Releases will have to wait if he has to make them.

I had a discussion with a Plone webmaster who manages sites where editors tend to 'overdo' their raw html editing skills.

Enabling the style attribute in Plone 5.X opens a can of worms compared to Plone 4. The HTML filter in Plone 4 has an extra tab 'style' in the control panel where the values of the styles attribute can be limited.

That filter-tab is no longer present in Plone 5. iirc the html filter module was rewritten in Plone 5 to use lxml natively. Apparently this was never ported/re-implemented.

I'm all for fixing the styling issues with TinyMCE, but I'm not sure if re-activating the styles attribute by default without the possibility to filter on attribute values in a Plone point release (5.2.2) is wise.

Good point.
My guess is that there are more people who benefit from having a TinyMCE with more functioning buttons, than there are people who are annoyed by not having all styles removed.
Workaround for the second group, is to remove style from the custom attributes in those sites after upgrading.

If we would want to re-add the whitelist of permitted style properties, here are some notes:

  • The Plone safe html filter calls the Cleaner class from lxml.
  • This Cleaner already removes the most dangerous styling, from a security point of view.
  • After returning from the Cleaner call, we could do something similar to what the Cleaner does with inline styles: find the styled elements, get their style attribute, which is just text, and clean it up.
  • Cleaning it up is probably: split by semicolon, split by colon into key/value, throw the key/value away unless the key is in the whitelist, set the changed style on the element.
  • The whitelist would be a new part of the IFilterSchema.

Then again, the SafeHTML transform has this remark in the doc string:

We only want security related filtering here, all the rest has to be done in TinyMCE & co.

The remark is from this commit three years ago
I don't know how we would do such style whitelisting in TinyMCE.
But then: if you disable the code tool from the TinyMCE toolbar, this may be enough.

(Bonus points if you can somehow enable the source code tool only for Site Administrators, or maybe make it read-only.)

I did some more checking and discovering this week and got some valuable feedback from my webmaster customer/contact. It's straight forward to change the definitions for alignment in TinyMCE.

Tiny even does this in the example they have online in the documentation for custom formats at https://www.tiny.cloud/docs/demo/format-custom/

{"alignleft": {
    "classes": "text-left", "block": "p"},
"aligncenter": {
    "classes": "text-center", "block": "p"},
"alignright": {
    "classes": "text-right", "block": "p"}
}

alignleft is a built-in definition to which the TinyMCE formatting menu, buttons etc. refer by name. If you override it like above, Tiny will use the class and not fill the style attribute.

The css-classes used are 1:1 matching bootstrap 3-4 classes for alignment. With the current efforts for Plone 6 to modernize Barceloneta to a new 'bootstrap 4 classes only' theme , this is isomething that can also be taken into account.

I also checked more on use of the 'style' attribuut in general. Apparently before IE 11 the style tag was not only a layout/visual but also security concern in some browsers for extranet/public sites. You could import css and do other nasty stuff. This was before CSP (content security policy) was introduced.

The Cleaner class in lxml still removes these kind of security related css content in the style attribute, also in current Plone 5.2 if I undestand correcty from Maurits' checks and previous post.

FYI there are some i18n issues in the TinyMCE link and image plugins: https://github.com/plone/Products.CMFPlone/issues/3120

Plone Foundation Code of Conduct