Ticket #8463 (closed Bug: fixed)

Opened 3 years ago

Last modified 10 months ago

AttributeError: kupu_captioned_image in Plone 3.x

Reported by: grahamperrin Owned by: ldr
Priority: major Milestone: 3.3.5
Component: Templates/CSS Keywords: patch
Cc: moldy, tom_gross, anthonygerrard, hoss, deesto

Description

Change History

comment:1 Changed 3 years ago by ldr

  • Owner set to ldr

comment:2 in reply to: ↑ description Changed 3 years ago by moldy

Replying to grahamperrin:

Discussed at  http://n2.nabble.com/AttributeError%3A-kupu_captioned_image-in-Plone-3.1.5.1-tp1075332p1075332.html

Thanks to Laurence Rowe, and I'll try his fix ASAP.

I just tested Laurence's fix. It works for me.

comment:3 in reply to: ↑ description ; follow-up: ↓ 4 Changed 3 years ago by grahamperrin

  • Cc moldy added

I backed up then patched
/opt/Plone-3.1/zeocluster/parts/plone/CMFPlone/skins/plone_content/folder_listing.pt

but still I see

  • AttributeError: kupu_captioned_image

and now, too

  • KeyError: 'text'

I'm not confident that my copy and unwrapping of text, from the support forum, produced a sane chunk of text when pasted into my /opt/Plone-3.1/zeocluster/parts/plone/CMFPlone/skins/plone_content/folder_listing.pt

Is < http://paste.plone.org/23668> representative of how the patched file should appear?

Feel free to correct at the pasteboard :)

Thanks again
Graham

comment:4 in reply to: ↑ 3 Changed 3 years ago by moldy

Replying to grahamperrin:

Is < http://paste.plone.org/23668> representative of how the patched file should appear?

You missed a change on line 38. Replace

tal:replace="structure text"

with

tal:replace="structure here/getText"

Here is a copy of the file that works for me:  http://rafb.net/p/vaZg8o43.html

comment:5 follow-up: ↓ 6 Changed 3 years ago by grahamperrin

OK ...

I copied from RAFB, patched then restarted my cluster.

For the four URLs below:

  • one or two page reloads (Safari 3.1.2 (5525.20.1) on Mac OS X 10.5.4) were required before errors ceased

< http://centrim.mis.brighton.ac.uk/research/past/folder_summary_view> < http://centrim.mis.brighton.ac.uk/research/present/folder_summary_view > < http://centrim.mis.brighton.ac.uk/research/past/folder_tabular_view> < http://centrim.mis.brighton.ac.uk/research/present/folder_tabular_view>

For the two URLs below:

  • I see gremlins, presumably a side effect of copying from RAFB

< http://centrim.mis.brighton.ac.uk/research/past/folder_listing> < http://centrim.mis.brighton.ac.uk/research/present/folder_listing>

I'll zap the gremlins in TextWrangler, re-patch, re-test and report.

comment:6 in reply to: ↑ 5 Changed 3 years ago by grahamperrin

I'll zap the gremlins in TextWrangler, re-patch, re-test and report.

Done and done.

Now:

  • folder_listing

displays seem to be fine, but both

  • folder_summary_view

and

  • folder_tabular_view

initially resulted in

  • AttributeError: kupu_captioned_image

and for reference I am pasting below two of the four tracebacks, but Safari page reloads seem to have worked around that symptom so I guess that there was some caching issue.

Thanks again
Graham


Traceback (innermost last):
  Module ZPublisher.Publish, line 119, in publish
  Module ZPublisher.mapply, line 88, in mapply
  Module ZPublisher.Publish, line 42, in call_object
  Module Shared.DC.Scripts.Bindings, line 313, in __call__
  Module Shared.DC.Scripts.Bindings, line 350, in _bindAndExec
  Module Products.CMFCore.FSPageTemplate, line 216, in _exec
  Module Products.CMFCore.FSPageTemplate, line 155, in pt_render
  Module Products.PageTemplates.PageTemplate, line 89, in pt_render
  Module zope.pagetemplate.pagetemplate, line 117, in pt_render
   - Warning: Macro expansion failed
   - Warning: exceptions.KeyError: 'fl_macro'
  Module zope.tal.talinterpreter, line 271, in __call__
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 891, in do_useMacro
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 536, in do_optTag_tal
  Module zope.tal.talinterpreter, line 521, in do_optTag
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 957, in do_defineSlot
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 536, in do_optTag_tal
  Module zope.tal.talinterpreter, line 521, in do_optTag
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 861, in do_defineMacro
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 957, in do_defineSlot
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 536, in do_optTag_tal
  Module zope.tal.talinterpreter, line 521, in do_optTag
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 534, in do_optTag_tal
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 949, in do_defineSlot
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 861, in do_defineMacro
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 536, in do_optTag_tal
  Module zope.tal.talinterpreter, line 521, in do_optTag
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 861, in do_defineMacro
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 586, in do_setLocal_tal
  Module zope.tales.tales, line 696, in evaluate
   - URL: file:/opt/Plone-3.1/zeocluster/parts/plone/CMFPlone/skins/plone_content/folder_summary_view.pt
   - Line 31, Column 8
   - Expression: <PathExpr standard:u'here/aq_explicit/getText|nothing'>
   - Names:
      {'container': <PloneSite at /centrim>,
       'context': <ATTopic at /centrim/research/past>,
       'default': <object object at 0x301528>,
       'here': <ATTopic at /centrim/research/past>,
       'loop': {},
       'nothing': None,
       'options': {'args': ()},
       'repeat': <Products.PageTemplates.Expressions.SafeMapping object at 0x94b1940>,
       'request': <HTTPRequest, URL=http://centrim.mis.brighton.ac.uk/research/past/folder_summary_view>,
       'root': <Application at >,
       'template': <FSPageTemplate at /centrim/folder_summary_view used for /centrim/research/past>,
       'traverse_subpath': [],
       'user': <SpecialUser 'Anonymous User'>}
  Module zope.tales.expressions, line 217, in __call__
  Module Products.PageTemplates.Expressions, line 161, in _eval
  Module Products.PageTemplates.Expressions, line 123, in render
  Module Products.Archetypes.ClassGen, line 56, in generatedAccessor
  Module wicked.fieldevent, line 29, in render
  Module zope.event, line 23, in notify
  Module zope.component.event, line 26, in dispatch
  Module zope.component._api, line 130, in subscribers
  Module zope.component.registry, line 290, in subscribers
  Module zope.interface.adapter, line 535, in subscribers
  Module wicked.fieldevent, line 16, in notifyFieldEvent
  Module zope.component._api, line 101, in getMultiAdapter
  Module zope.component._api, line 114, in queryMultiAdapter
  Module zope.component.registry, line 206, in queryMultiAdapter
  Module zope.interface.adapter, line 482, in queryMultiAdapter
  Module wicked.fieldevent.meta, line 78, in field_value
  Module Products.Archetypes.Field, line 1391, in get
  Module Products.Archetypes.BaseUnit, line 100, in transform
  Module Products.PortalTransforms.TransformEngine, line 179, in convertTo
  Module Products.PortalTransforms.chain, line 46, in convert
  Module Products.PortalTransforms.Transform, line 190, in convert
  Module Products.kupu.plone.html2captioned, line 127, in convert
AttributeError: kupu_captioned_image
Traceback (innermost last):
  Module ZPublisher.Publish, line 119, in publish
  Module ZPublisher.mapply, line 88, in mapply
  Module ZPublisher.Publish, line 42, in call_object
  Module Shared.DC.Scripts.Bindings, line 313, in __call__
  Module Shared.DC.Scripts.Bindings, line 350, in _bindAndExec
  Module Products.CMFCore.FSPageTemplate, line 216, in _exec
  Module Products.CMFCore.FSPageTemplate, line 155, in pt_render
  Module Products.PageTemplates.PageTemplate, line 89, in pt_render
  Module zope.pagetemplate.pagetemplate, line 117, in pt_render
   - Warning: Macro expansion failed
   - Warning: exceptions.KeyError: 'fl_macro'
  Module zope.tal.talinterpreter, line 271, in __call__
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 891, in do_useMacro
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 536, in do_optTag_tal
  Module zope.tal.talinterpreter, line 521, in do_optTag
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 957, in do_defineSlot
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 536, in do_optTag_tal
  Module zope.tal.talinterpreter, line 521, in do_optTag
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 861, in do_defineMacro
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 957, in do_defineSlot
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 536, in do_optTag_tal
  Module zope.tal.talinterpreter, line 521, in do_optTag
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 534, in do_optTag_tal
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 949, in do_defineSlot
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 861, in do_defineMacro
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 536, in do_optTag_tal
  Module zope.tal.talinterpreter, line 521, in do_optTag
  Module zope.tal.talinterpreter, line 516, in no_tag
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 861, in do_defineMacro
  Module zope.tal.talinterpreter, line 346, in interpret
  Module zope.tal.talinterpreter, line 586, in do_setLocal_tal
  Module zope.tales.tales, line 696, in evaluate
   - URL: file:/opt/Plone-3.1/zeocluster/parts/plone/CMFPlone/skins/plone_content/folder_tabular_view.pt
   - Line 33, Column 8
   - Expression: <PathExpr standard:u'here/aq_explicit/getText|nothing'>
   - Names:
      {'container': <PloneSite at /centrim>,
       'context': <ATTopic at /centrim/research/past>,
       'default': <object object at 0x301528>,
       'here': <ATTopic at /centrim/research/past>,
       'loop': {},
       'nothing': None,
       'options': {'args': ()},
       'repeat': <Products.PageTemplates.Expressions.SafeMapping object at 0x94a9cd8>,
       'request': <HTTPRequest, URL=http://centrim.mis.brighton.ac.uk/research/past/folder_tabular_view>,
       'root': <Application at >,
       'template': <FSPageTemplate at /centrim/folder_tabular_view used for /centrim/research/past>,
       'traverse_subpath': [],
       'user': <SpecialUser 'Anonymous User'>}
  Module zope.tales.expressions, line 217, in __call__
  Module Products.PageTemplates.Expressions, line 161, in _eval
  Module Products.PageTemplates.Expressions, line 123, in render
  Module Products.Archetypes.ClassGen, line 56, in generatedAccessor
  Module wicked.fieldevent, line 29, in render
  Module zope.event, line 23, in notify
  Module zope.component.event, line 26, in dispatch
  Module zope.component._api, line 130, in subscribers
  Module zope.component.registry, line 290, in subscribers
  Module zope.interface.adapter, line 535, in subscribers
  Module wicked.fieldevent, line 16, in notifyFieldEvent
  Module zope.component._api, line 101, in getMultiAdapter
  Module zope.component._api, line 114, in queryMultiAdapter
  Module zope.component.registry, line 206, in queryMultiAdapter
  Module zope.interface.adapter, line 482, in queryMultiAdapter
  Module wicked.fieldevent.meta, line 78, in field_value
  Module Products.Archetypes.Field, line 1391, in get
  Module Products.Archetypes.BaseUnit, line 100, in transform
  Module Products.PortalTransforms.TransformEngine, line 179, in convertTo
  Module Products.PortalTransforms.chain, line 46, in convert
  Module Products.PortalTransforms.Transform, line 190, in convert
  Module Products.kupu.plone.html2captioned, line 127, in convert
AttributeError: kupu_captioned_image

comment:7 Changed 3 years ago by grahamperrin

For reference only, < http://paste.plone.org/23672> is hopefully gremlin-free.

comment:8 Changed 3 years ago by grahamperrin

  • Component changed from Unknown to Visual and templates
  • Summary changed from AttributeError: kupu_captioned_image in Plone 3.1.5.1 to AttributeError: kupu_captioned_image - folder_listing.pt in Plone 3.1.5.1

comment:9 Changed 3 years ago by grahamperrin

Possible cross-reference ticket:8475

comment:10 Changed 3 years ago by grahamperrin

< http://centrim.mis.brighton.ac.uk/research/past/>, a collection

  • summary display was good on first load, but a page refresh (2008/09/10 07:46:58.868 GMT+1) results in

AttributeError: kupu_captioned_image

  • tabular display is currently OK even when refreshed
  • standard display is currently OK even when refreshed.

comment:11 follow-up: ↓ 12 Changed 3 years ago by fairsay

I had the same problem but patching folder_listing.pt didn't solve it. Instead I changed line 35 of folder_summary_view.pt from:

text here/aq_explicit/getText|nothing

to

text here/getText|nothing

Then the problem seems to have been solved. However I don't know if this simply solves it for this case (using a Collection as the default folder item and making the folder summary view as the default Collection view) and might create problems for using the folder_summary_view.pt in the non-Collection folder view.

I haven't checked this into SVN because I'll let more competent programmers (I merely fiddle with the code) and also because I don't have Plone SVN access (only collective access)

Hopefully this helps others solve the immediate problem with using the folder_summary_view of a Collection as the default view and helps people who know what they are doing asses if it is the proper solution :-)

comment:12 in reply to: ↑ 11 ; follow-up: ↓ 14 Changed 3 years ago by fairsay

To clarify the above post: patching folder_listing.pt didn't solve my problem because I was using folder_summary_view.pt (as it seems others were) on a collection (as it seems others were).

The change I made above seems to be holding for both Collections and 'normal' folders.

comment:13 Changed 3 years ago by tom_gross

  • Cc tom_gross added

comment:14 in reply to: ↑ 12 ; follow-up: ↓ 15 Changed 3 years ago by fairsay

Replying to fairsay:

To clarify the above post: patching folder_listing.pt didn't solve my problem because I was using folder_summary_view.pt (as it seems others were) on a collection (as it seems others were).

The change I made above seems to be holding for both Collections and 'normal' folders.

Further update (and now using Plone 3.2.1 where the issue still exists), making the above change makes the error on setting a collection view as the default (in my case on the site homepage) go away (as reported) but seems to then make it impossible to view the collection /folder_contents. The traceback is below:

Traceback (innermost last):

Module ZPublisher.Publish, line 119, in publish Module ZPublisher.mapply, line 88, in mapply Module ZPublisher.Publish, line 42, in call_object Module Products.Five.browser.metaconfigure, line 417, in call Module Shared.DC.Scripts.Bindings, line 313, in call Module Shared.DC.Scripts.Bindings, line 350, in _bindAndExec Module Products.PageTemplates.PageTemplateFile, line 129, in _exec Module Products.CacheSetup.patch_cmf, line 120, in PT_pt_render

  • Warning: Macro expansion failed
  • Warning: exceptions.AttributeError: widget

Module zope.tal.talinterpreter, line 271, in call Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 891, in do_useMacro Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 536, in do_optTag_tal Module zope.tal.talinterpreter, line 521, in do_optTag Module zope.tal.talinterpreter, line 516, in no_tag Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 957, in do_defineSlot Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 536, in do_optTag_tal Module zope.tal.talinterpreter, line 521, in do_optTag Module zope.tal.talinterpreter, line 516, in no_tag Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 861, in do_defineMacro Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 957, in do_defineSlot Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 536, in do_optTag_tal Module zope.tal.talinterpreter, line 521, in do_optTag Module zope.tal.talinterpreter, line 516, in no_tag Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 534, in do_optTag_tal Module zope.tal.talinterpreter, line 516, in no_tag Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 949, in do_defineSlot Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 534, in do_optTag_tal Module zope.tal.talinterpreter, line 516, in no_tag Module zope.tal.talinterpreter, line 346, in interpret Module zope.tal.talinterpreter, line 745, in do_insertStructure_tal Module Products.PageTemplates.Expressions, line 221, in evaluateStructure Module zope.tales.tales, line 696, in evaluate

  • URL: index
  • Line 63, Column 8
  • Expression: <PathExpr standard:'view/contents_table'>
  • Names:

{'container': <PloneSite at /FairSay>,

'context': <PloneSite at /FairSay>, 'default': <object object at 0xb7f2c528>, 'here': <PloneSite at /FairSay>, 'loop': {}, 'nothing': None, 'options': {'args': (<Products.Five.metaclass.SimpleViewClass from /home/fairsay/webapps/primary/eggs/plone.app.content-1.3-py2.4.egg/plone/app/content/browser/foldercontents.pt object at 0x116e7eac>,)}, 'repeat': <Products.PageTemplates.Expressions.SafeMapping object at 0x12f36c2c>, 'request': <HTTPRequest, URL= http://fairsay.com/folder_contents>, 'root': <Application at >, 'template': <ImplicitAcquirerWrapper object at 0x12f2daec>, 'traverse_subpath': [], 'user': <PropertiedUser 'duane'>, 'view': <Products.Five.metaclass.SimpleViewClass from /home/fairsay/webapps/primary/eggs/plone.app.content-1.3-py2.4.egg/plone/app/content/browser/foldercontents.pt object at 0x116e7eac>, 'views': <zope.app.pagetemplate.viewpagetemplatefile.ViewMapper object at 0x12657d2c>}

Module zope.tales.expressions, line 217, in call Module Products.PageTemplates.Expressions, line 161, in _eval Module Products.PageTemplates.Expressions, line 123, in render Module plone.app.content.browser.foldercontents, line 28, in contents_table Module plone.app.content.browser.foldercontents, line 91, in init Module plone.memoize.instance, line 45, in memogetter Module plone.app.content.browser.foldercontents, line 152, in items Module urllib, line 1122, in quote_plus

TypeError: iterable argument required

comment:15 in reply to: ↑ 14 Changed 3 years ago by fairsay

  • Keywords collection, default view, folder_summary_view added

Ignore the last (mangled traceback) post above.

After wiping and rebuilding Plone (3.2.2) from scratch (using buildout) and only making a change to folder_summary_view.pt, the issue with the folder summary view of a collection being made the default view is resolved and the folder_contents view is still working.

Any chance of this 'fix' being checked by someone who knows the implications and, it it is the correct fix, checking it into Plone core? Seems silly for such a bug to linger for so long :-)

comment:16 Changed 3 years ago by Linux_Blade_Guy

I'm having this problem with atct_album_view under Plone 3.2.1 as well... I'll try the proposed change above and see what happens...

comment:17 Changed 3 years ago by Linux_Blade_Guy

  • Summary changed from AttributeError: kupu_captioned_image - folder_listing.pt in Plone 3.1.5.1 to AttributeError: kupu_captioned_image in Plone 3.x

Seems to reliably fix atct_album_view as well, or at least I haven't been able to reproduce the problem since making the change. All I did was change line 38 of atct_album_view from:

                     here/aq_explicit/getText|nothing"

to

                     text here/getText|nothing"

Can anyone comment on any potential problems with this?

comment:18 Changed 3 years ago by hannosch

  • Component changed from Visual and templates to Templates/CSS

comment:19 Changed 3 years ago by dunlapm

Looking into the actaul error, it appears that kupu's transform assumes it can aquire kupu_captioned_image by simply asking for context.kupu_captioned_image. However, if we're using context/aq_explicit/getText, then aquisition won't work. I'm pretty sure this aq_explicit is so that folders/collections with text defined won't get aquired (much in the same way default content can be aquired if you delete the default view but do no unset the default_page property).

So the question is, can kupu get the "kupu_captioned_image" property through some method other than direct acquisition off of context, or should the call to aq_explicit be replace by something like aq_base, or as suggested above, test for the existance of aq_explicit/getText before using getText, and avoid aq_explicit?

comment:20 Changed 3 years ago by sergey_v

This aq_explicit is actually a fix for another bug -  https://dev.plone.org/plone/ticket/8190 - the problem being that the templates are used by both Folders and Topics but only Topics have 'text' field, so Folders may acquire the attribute from its parent and display it (and you can't edit it, which is rather confusing).

So the proper fix would probably be first to test if the attribute actually exists and then get it without using aq_explicit (as in Laurence post here -  http://plone.org/support/forums/general#nabble-td1075332|a1075332)

comment:21 Changed 3 years ago by anthonygerrard

  • Cc anthonygerrard added

comment:22 Changed 3 years ago by hoss

  • Cc hoss added

comment:23 Changed 3 years ago by hoss

Any official update on the status of this bug? The is a fairly severe bug, and seems like it ought not to have been this long-lived.

It also strikes me that this shouldn't necessarily be fixed in template-land. The offending line in html2captioned.py is:

template = context.kupu_captioned_image

That's done without regard for whether or not (a) context is None, and (b) kupu_captioned_image is a valid property of context. Both of those are bad, and lead me to think that the implementation of convert() isn't taking the right perspective, and that perhaps the original tweaks to the folder templates (via #8190) were themselves ill-advised.

Acquisition exists for a reason, and should be allowed to function as designed. In general, when a situation occurs where acquisition is undesired - should it be blocked in the template or blocked in the object being accessed? I don't know for sure, but my gut tells me that kind of thing shouldn't be happening in templates.

And unfortunately none of this explains why kupu is in the critical path for rendering either folder or topic contents. I think this also suggests that a clean abstraction is missing for whatever feature is involved here that is injecting kupu code into the mix (presumably image captions).

I suggest that the Component for this bug be changed to 'Visual Editor (Kupu)', and that a robust solution be approached from that direction.

comment:24 Changed 3 years ago by hoss

Here's an alternative proposed fix for this. It changes kupu's html2captioned.py to be insensitive to whether or not the 'context' object pulled out of kwargs is acquisition-wrapped. I believe that is the heart of this bug: convert() is implemented with the implicit assumption that the 'context' it is given is an acquisition-wrapped object, when under some circumstances (namely anytime any of the folder based templates are used) the context has been purposefully unwrapped (via aq_explicit).

So instead of trying to obtain via 'context.<property name>' the handful of things that the functions needs that aren't actually properties of 'context', and relying on acquisition to go upstream to get them, the function doesn't use 'context' to ask for them in the first place. It goes straight to something that is both (a) known to exist and (b) known to be capable of getting the items that are themselves known to exist: namely the root site object, from which the kupu_captioned_image template and the archetype_tool can be obtained.

Here's the change:

26a27
> from zope.app.component.hooks import getSite
127,130c128,133
<         template = context.kupu_captioned_image
<         if context is not None:
<             at_tool = context.archetype_tool
<             rc = at_tool.reference_catalog
---
> 
>         site = getSite()
>         template = site.portal_skins.kupu_plone.kupu_captioned_image
> 
>         at_tool = site.archetype_tool
>         rc = at_tool.reference_catalog

As you can see, it's pretty straightforward:

  • getSite() is imported so that convert() can use it
  • the site object is obtained from getSite()
  • the site object is then leveraged to obtain the template object (known to exist as portal_skins.kupu_plone.kupu_captioned_image)
  • the site object is also leveraged to get the archetype_tool (this was a lurking hidden bug identical in nature to the kupu_captioned_image problem)

It then proceeds as it did normally. There may or may not be additional considerations necessary to ensure this approach is robust under all circumstances. But I firmly believe it is the more appropriate fix for this problem. Unless, that is, indirectly invoking convert() on objects unwrapped (as they are in folder_listing.pt, etc..) should not be allowed. If that is the case, then convert() should validate the nature of 'context' upon invocation.

comment:25 Changed 2 years ago by jonstahl

  • Keywords patch added; collection, default view, folder_summary_view removed

comment:26 Changed 2 years ago by davisagli

  • Status changed from new to closed
  • Resolution set to fixed

(In [30842]) Adjust the fix for container text getting acquired (refs #8190) to make sure that we call getText with a full acquisition chain, for the benefit of kupu's captioning transform (fixes #8463). Kupu should probably be fixed too, but I'm making the change here for expediency's sake.

comment:27 Changed 2 years ago by vangheem

  • Status changed from closed to reopened
  • Resolution fixed deleted

Hoss is right on this--4 months ago now. We need a better fix for this and we need it fixed in the convert method of html2captioned. We can do what he describes or attempt to traverse to it.

Right now this can cause errors to occur when renaming a plone object in the zmi.

comment:28 Changed 2 years ago by anthonygerrard

I've just been testing this on our server and it now seems like the text field is not displayed at all on the standard, summary and tabular views of a collection. It is displayed on the thumbnail and collection views

comment:29 Changed 23 months ago by rockdj

This issue appears to be one I've just hit whilst using and trying to render a plone.app.textfield.value.RichTextValue (via Dexterity, FWIW). A portal_transforms call to its convertTo method eventually ends up in html2captioned's convert method and since there's no context been specified, the system falls over.

Specifying a context solves this specific issue (about to open an associated ticket). However, the underlying code (either in html2captioned or elsewhere) should either be more explicit about actually requiring a context rather than just assuming one has been passed in.

comment:30 Changed 19 months ago by deesto

  • Cc deesto added

Was this ever resolved officially? I just caught this bug out of the blue in a Collection object (which happens to have no image attached) in a 3.1.7 instance, and the error persisted on this object for weeks, until I opened the object for editing, and saved it without making any changes.

comment:31 Changed 19 months ago by deesto

I shouldn't be so trusting in magic resolutions without more thorough testing: actually, in my case, this crashes with the kupu image error:

object/view

... while this presents the page normally:

object

I guess that verifies a view template problem.

comment:32 Changed 19 months ago by deesto

... and I can confirm that manually folding in the "fixes" in [30842] (which is for Plone 3.3) into a 3.1.7 site breaks more stuff than it fixes (obviously).

comment:33 Changed 19 months ago by maurits

  • Status changed from reopened to closed
  • Resolution set to fixed

I'm looking at this. Since it was not obvious to me how to reproduce this error in Plone 3.3.5, here are the steps:

  • Go to Site Setup -> Visual Editor: enable 'Allow captioned images'. Possibly enable 'Link using UIDs' as well.
  • Site Setup -> Site Setting: check 'Enable inline editing'. Not strictly necessary it seems, but I have seen cases when just viewing a Collection was fine, but clicking to edit the text inline gave the reported AttributeError: kupu_captioned_image
  • To also check #8190 (getText of a parent is shown when the current context has no text field): go to the ZMI -> portal_types -> Topic (Collection): allow adding Folders.
  • Now edit the standard news collection at news/aggregator and give it some text (there is no need to add a picture).

What can go wrong at this point?

  • When the folder_summary_view is selected as display method, no text is shown. Same for folder_listing and folder_tabular_view. This has been fixed in Products.CMFPlone r37779 (see #8190). Should appear in a future 3.3.6 release.
  • With the album view, for some reason after clicking 'Save' the Collection may look fine, but sometimes you get an AttributeError: kupu_captioned_image. Saving it a second time may help (or the other way around...). That is quite strange. I don't know what can cause this.
  • The AttributeError is caused by some acquisition weirdness. The previously mentioned ticket #8190 has some fixes for this, but misses a few templates. Now fixed for the album view in Products.ATContentTypes in http://dev.plone.org/collective/changeset/120628 Should appear in a future 1.3.7 release.

comment:34 follow-up: ↓ 35 Changed 19 months ago by deesto

Great; thank you Maurits. But what about existing instances, many of which are broken?

comment:35 in reply to: ↑ 34 ; follow-up: ↓ 36 Changed 19 months ago by maurits

Replying to deesto:

Great; thank you Maurits. But what about existing instances, many of which are broken?

Existing instances should just update to Plone 3.3.6 once that is released, or copy over the latest versions of folder_summary_view.pt and friends from the Plone 3.3 branch into an own custom package or into the custom folder in portal_skins. Or create a private release of Plone and ATContentTypes.

Unless I am missing something, that should solve it. And apparently in some cases it weirdly also works to edit and save an item again, but only when you are lucky so don't count on it.

Note that there will never be a Plone 3.1.8 bugfix release, if that is what you mean: the Plone 3 series is considered stable enough that updating from Plone 3.1.7 to Plone 3.3.5 should work fine without backwards compatibility issues for add-on products (unless they do very unexpected/bad stuff).

comment:36 in reply to: ↑ 35 ; follow-up: ↓ 37 Changed 12 months ago by deesto

  • Status changed from closed to reopened
  • Resolution fixed deleted

Replying to maurits:

Replying to deesto:

Great; thank you Maurits. But what about existing instances, many of which are broken?

Existing instances should just update to Plone 3.3.6 once that is released ... Note that there will never be a Plone 3.1.8 bugfix release, if that is what you mean: the Plone 3 series is considered stable enough that updating from Plone 3.1.7 to Plone 3.3.5 should work fine without backwards compatibility issues for add-on products (unless they do very unexpected/bad stuff).

This isn't really a fair assumption to make: 3.1.7 was the last non-buildout based version of Plone. 3.3.6 is buildout-based and therefore isn't completely backwards-compatible.

Is there a simple fix for non-buildout instances, one that does not require the creation of a custom package or a "private" Plone release?

comment:37 in reply to: ↑ 36 ; follow-up: ↓ 38 Changed 12 months ago by maurits

  • Status changed from reopened to closed
  • Resolution set to fixed

Replying to deesto:

Is there a simple fix for non-buildout instances, one that does not require the creation of a custom package or a "private" Plone release?

In the portal_skins you can search for the affected templates and customize them: folder_listing, folder_summary_view, folder_tabular_view, and atct_album_view.

In those templates make the changes listed here: r37779 and here: http://dev.plone.org/collective/changeset/120628

comment:38 in reply to: ↑ 37 Changed 12 months ago by deesto

Replying to maurits:

Replying to deesto:

Is there a simple fix for non-buildout instances, one that does not require the creation of a custom package or a "private" Plone release?

In the portal_skins you can search for the affected templates and customize them: folder_listing, folder_summary_view, folder_tabular_view, and atct_album_view.

In those templates make the changes listed here: r37779 and here: http://dev.plone.org/collective/changeset/120628

Thanks Maurits. I should have also requested a solution that did not require monkey-patching the base code of an official Plone release.

comment:39 Changed 12 months ago by hoss

I'm dismayed that the approach to fixing this problem continues involve changing templates.

It's not a template bug. It's a kupu bug.

I described a fix that, as far as I know, completely resolves the issue under all circumstances, regardless of what template (old, new, or custom) happens to cause convert() to be invoked.

A bug in a python function should be fixed in the python function. Making piecemeal hacks to templates doesn't solve the problem, it just leaves it lurking for someone else to stub their toe on.

comment:40 Changed 10 months ago by keul

We found that some of our old sites can fall into this bug.

We released a port of the fix ( experimental.collectionfix8463) is someone will never need it.

comment:41 Changed 10 months ago by hoss

I'm confused why a perfectly serviceable fix for this problem continues to be uniformly ignored in favor of a brittle and non-comprehensive set of tweaks to templates, which is incompatible with any site that may have chosen to customize those templates.

Very disheartening.

I found a programmatic fix for this bug within about an hour of encountering it (nearly 2 years ago), and dutifully described it in detail on this thread. I've had it applied on my sites ever since then, without any problems.

Note: See TracTickets for help on using tickets.