Hi guys,

Below is the review for PLIP 149 - Improved Markup Support

http://plone.org/products/plone/roadmap/149

What is it?
-----------

- A way of registering additional transformations to support different text transforms more easily, without requiring the content type (i.e. the AT TextField and RichWidget) to be aware of that markup type up-front.

- A control panel to support this

Who did it?
-----------

Tom Lazar ([EMAIL PROTECTED]) did the majority of this work.

How do I check it out?
----------------------

I ran the bundle on Zope 2.10 svn:

svn co svn.plone.org/svn/plone/review/plip-149-markup-support/

After installing this, you will be able to select additional markups in the MIME-type selection drop-down when editing a Page or other content object.

You will also see a new configlet in the control panel for selecting the default markup type and selecting the additional types that are available.

There are branches of CMFPlone (for the configlet), PortalTransforms (for the new transforms), Archetypes (to support non-hardcoded lists of available input MIME types) and ATContentTypes (to let ATCT's types elect to use the dynamic transforms).

Implementation
--------------

o A method listAvailableTextInputs() on the portal_transform tool returns all transforms that can transform from text/ to HTML.

o A method getAllowedContentTypes() on Field in AT is used by the widget macro to determine which content types may be selected. If the field has set allowed_content_types explicitly, this always wins.

However, if it's not set (as it is now not on the ATCT branch), the method getAllowedContentTypes() in Products.Archetypes.utils is called instead. This queries the allowable types (as per listAvailableTextInputs()) and then removes any blacklisted types as managed by the configlet.

Like many of our configlets, it gives the illusion of managing a whitelist in the UI, whilst the implementation is a blacklist. This means that when a new product is installed providing a new markup, it doesn't need to be explicitly activated.

Concerns
---------

o Markup and Textile support requires additional Python modules to be installed. They should probably not be available for selection unless these modules are installed.

o In fact, things like python source etc. should be disabled by default. This would be a fairly simple matter of adding the appropriate property initialisation, however.

o The configlet is called 'Type Settings' which is too generic. It also has some strange language and references AT details like TextField. All easy to fix, of course.

o There doesn't appear to be a way to manage markups per-content-type, which would be useful. I still think this is valuable even if that isn't possible, though.

o It's unclear to me whether/how this corresponds to the work on Wicked

o Looking at http://dev.plone.org/archetypes/changeset/6957, it's not clear to me why we need to set the field's allowable content types from the site property that holds the default values as managed by the configlet.

o The template for the configlet does things like allowable_types python:modules['Products.Archetypes.mimetype_utils'].getAllowableContentTypes(here); It really could benefit from using a view!

Recommendation and caveats
---------------------------

I think that this could benefit from some code cleanup and fine tuning, but that it's generally quite solid. Having supported input MIME types hardcoded in every RichWidget-using field by default is really an anti-pattern, and this just decouples that (note that fields that *do* explicitly state their MIME types will not be affected by this - to get the new behaviour, you have to *not* set allowable_content_types on the field).

This is obviously very AT specific, and there is no use of any Z3 tech here. Using a view for the configlet would be a trivial, but useful improvement. Using utilities to manage the actual available transforms and using a local utility rather than site_properties to store the default and disabled transforms would seem to be more modern patterns.

However, this PLIP was never about radically refactoring PortalTransforms, and the changes it introduces are fairly minor. I don't think you could reasonably make transforms be utilities the way PortalTransforms works today, nor do we have any consideration yet of transform support in non-AT objects (unless they explicitly choose to invoke portal_transform, in which case they could also refer to the properties used in this bundle for their available and default MIME types). If/when a more general refactoring of PortalTransforms happens, refactoring the work done by this PLIP to take advantage of it should be simple.

The one missing feature is the ability to manage available MIME types per-content-type. This would more likely require something like a local utility to hold the settings and a custom UI to manage it. It probably wouldn't be very hard, but I feel that even without it, this functionality is useful. In most cases, I suspect that for the content types where you don't want to explicitly set the allowable types in code, it is because you want people to use the markup they are comfortable with. In that case, having the same set of markups for Documents, News Items and whatever else would be fine.

Another possible improvement may be to allow a personal selection of preferred markup. Again, not very hard, but also not criminal to delay.

Martin


_______________________________________________
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team

Reply via email to