p.a.blocks transforms change default behavior of ajax responses

[rnixx]

After installing plone.app.blocks, all AJAX HTML responses are wrapped within html / body tags, which differs how zope normally behaves and thus may break existing addons.

The responsible code line is here

https://github.com/plone/plone.app.blocks/blob/master/plone/app/blocks/transform.py#L84

Some unwanted side effect is described here

collective/collective.cover#331

how can we ensure the responsible transform only considers requests referring to p.a.blocks?

[rnixx]

Uplcoming questions about this issue

what's the reason of wrapping at all -> the ESI stuff?
where are the results processed further on the client side?

[rnixx]

As described here

https://github.com/plone/plone.transformchain/blob/master/plone/transformchain/interfaces.py#L15

binding the transform to the basic tile interface might do the job

i created a branch with this changes

9b6742e

and run the tests. Beside 2 failing tests in the main branch one more tests is failing.

If this is the desired fix for the issue, i'll fix the tests and merge the branch. comments appreciated

[vangheem]

I was just experiencing this issue.

Your fix is good and I think you should merge. Thanks!

[davisagli]

I don't understand the fix. The transforms are designed to operate on things that reference tiles (i.e. potentially any text/html response), not on the tiles themselves. Doesn't this change completely disable tile rendering?

[vangheem]

oh right.... Then how do we stop this?

[vangheem]

How I got around it is making my responses content-type text/xml and then the transform won't run on them...

[davisagli]

I think depending on a text/html response to not contain html and body tags is wrong, because without them it's not valid HTML. Seems like the response should be given a different mimetype. text/x-not-quite-html or something.

But if it's really necessary maybe the transform can detect when the original response doesn't have these tags, and not transform in that case.

[rnixx]

@davisagli referring to the documentation of ITransform, the multi adapter applies to the last element returned by traversal, which should be a view. that was the reason i thought binding to IBaseicTile does the job.

Referring to invalid HTML. Basically this argument does not matter at all. plone.app.blocks changes the way responses are delivered in general, thus forcing everybody else to change their code. Not everything is tiles or blocks, i even not understand yet what's the concrete reason for this transforms.

Some may enlighten me ;)

[davisagli]

The whole point of plone.app.blocks is that it allows you to reference tiles or layouts from any html response. So yes, it therefore needs to process all html responses to find out if those things are referenced. That is by design. Not everything is a tile, but anything may include a tile.

If that's not acceptable, you can a) not use plone.app.blocks, set request['plone.app.blocks.disabled'] = True to disable the transforms for your request, b) return a mimetype that won't get transformed, or d) find a way to abort the transform and return the original response if no tiles or layouts were found

[rnixx]

i.e., if i have a valid dom tree in the browser. and write some kind of ajaxified application which alters just a small part of the document (maybe a div or similar), and this small part gets delivered by an ordinary browser view, i don not see how this invalidates the overall document. I also don't know any specification which requires a valid HTML document structure for ajax snippets.

referring solution a. this might get difficult in future, i.e. i ran into this issue after installing collective.cover, and the decision using it is not necessarily made by the developer

as said above, p.a.blocks changes the way zope browser views behave by default, and that's a very very bad thing

[rnixx]

another question - whats wrong with doing the reference detection stuff without wrapping the response in a HTML document and leave document validity elsewhere?

[davisagli]

plone.app.blocks was developed with Deco in mind, where tiles can be included anywhere. But for collective.cover tiles only appear in the view of a cover. So perhaps collective.cover should not include the plone.app.blocks zcml but should instead register just the ParseXML and IncludeTiles transforms for covers only.

Turning the response into valid html is not an explicit goal of plone.app.blocks, but is something that happens incidentally as a result of reserializing the lxml tree with the html serializer after processing it to insert tile contents, merge panels, and include the layout. So it can't be avoided in the case where the response actually does need to be modified.

[rnixx]

it can be avoided with a custom HTML serialzer then (especially since wrapping is not a requirement for working blocks/tiles).

The point is still that p.a.blocks changes default zope behavior == BAD.

No matter how a workaround might look like right now. This should definitely be fixed. In my case, it breaks some of my own addon products, but i bet there's also lots of other 3rd party stuff effected.

[rnixx]

This is the html serializer of lxml.

https://github.com/lxml/lxml/blob/master/src/lxml/html/__init__.py#L1524

should be possible to avoid wrapping by several control flags, need to experiment a little bit

and here's the serializer creating utility. the serializer might even be initialized without using this utility.

https://github.com/repoze/repoze.xmliter/blob/master/repoze/xmliter/utils.py#L21