Hi! As also mentioned here <http://mail.zope.org/pipermail/cmf-checkins/2003-October/003702.html> some CMF tool tabs are broken if used with a current Zope-2_6-branch checkout. I didn't spend much time to find out what's really going on here, so I might be missing something. But it looks like the fix for <http://collector.zope.org/Zope/1058> (regarding an PropertySheets issue) causes this bug. Can anybody confirm this? Or has anybody an other idea which recent change could cause the problem? Was it really necessary to break backwards compatibility? Thanks, Yuppie
Yuppie wrote at 2003-10-15 14:24 +0200:
As also mentioned here <http://mail.zope.org/pipermail/cmf-checkins/2003-October/003702.html> some CMF tool tabs are broken if used with a current Zope-2_6-branch checkout.
I didn't spend much time to find out what's really going on here, so I might be missing something.
But it looks like the fix for <http://collector.zope.org/Zope/1058> (regarding an PropertySheets issue) causes this bug. Can anybody confirm this?
I fear I can confirm this. "App.Management.Tabs.manage_workspace" apparently had the intention to return "getattr(self,method)(self,REQUEST)" in the case that "method" did not contain a "/", and to redirect otherwise. Due to a bug in the the condition, however, it redirected for all methods unless they started with a "/" (reported as a bug in the Zope mailing list). I fixed the condition (patch to collector 1058). Unfortunately, returning "getattr(self,method)(self,REQUEST)" requires the method to be DTML like and breaks if this is not the case. I now think it is best to *always" use the redirect and never return the method. My code now looks like: # DM: fixing the wrong condition now reveals a bug in WingDBG # always redirect (as done earlier with the exception of # absolute URLs) if 1 or m.find('/') >= 0: # DM: let absolute URLs work (as generated by # "OFS.PropertySheets.PropertySheets.manage_options") prefix= m.startswith('/') and REQUEST['BASE0'] or REQUEST['URL1'] raise 'Redirect', ( "%s/%s" % (prefix, m)) return getattr(self, m)(self, REQUEST) Dieter
Hi! Dieter Maurer wrote:
Due to a bug in the the condition, however, it redirected for all methods unless they started with a "/" (reported as a bug in the Zope mailing list). I fixed the condition (patch to collector 1058).
Unfortunately, returning "getattr(self,method)(self,REQUEST)" requires the method to be DTML like and breaks if this is not the case.
I now think it is best to *always" use the redirect and never return the method. My code now looks like: [...]
Thanks! I don't have the time to look into the details. Meanwhile Andreas also stumbled over this issue (with different symptoms, see <http://article.gmane.org/gmane.comp.web.zope.plone.user/8507> ) and reverted the manage_workspace changes in CVS. Don't know - if someone checks in your new patch soon (I guess there should be unittests that catch the broken use cases so any future checkins are checked against these use cases.) - if collector #1058 should be reopened - if Zope is currently more broken than before because parts of your patch are still in CVS Cheers, Yuppie
Yuppie wrote at 2003-10-20 09:22 +0200:
Dieter Maurer wrote:
Due to a bug in the the condition, however, it redirected for all methods unless they started with a "/" (reported as a bug in the Zope mailing list). I fixed the condition (patch to collector 1058).
Unfortunately, returning "getattr(self,method)(self,REQUEST)" requires the method to be DTML like and breaks if this is not the case.
I now think it is best to *always" use the redirect and never return the method. My code now looks like: [...]
Thanks! I don't have the time to look into the details.
Meanwhile Andreas also stumbled over this issue (with different symptoms, see <http://article.gmane.org/gmane.comp.web.zope.plone.user/8507> ) and reverted the manage_workspace changes in CVS.
Don't know - if someone checks in your new patch soon (I guess there should be unittests that catch the broken use cases so any future checkins are checked against these use cases.)
I do not think it is useful to check something that definitely can no longer happen.
- if collector #1058 should be reopened
Let's see whether someone else needs PropertySheets outside ZClasses.
- if Zope is currently more broken than before because parts of your patch are still in CVS
This is unlikely. We now have again (as before) that (host relative) absolute URLs in "manage_options"' actions trigger an "AttributeError". Few people use absolute URLs in "manage_options". If they do, they immediately stop so as it does not work... PropertySheets use absolute URLS but apparently only me and someone else are interested in them. Dieter
In article <16274.18796.135508.105635@gargle.gargle.HOWL> you write:
Unfortunately, returning "getattr(self,method)(self,REQUEST)" requires the method to be DTML like and breaks if this is not the case.
Note that this particular problem can easily be cured by the standard if getattr(aq_base(ob), 'isDocTemp', 0) test (see CMFCore/PortalContent.py for an example). Florent -- Florent Guillaume, Nuxeo (Paris, France) +33 1 40 33 79 87 http://nuxeo.com mailto:fg@nuxeo.com
Florent Guillaume wrote at 2003-10-22 12:25 +0200:
In article <16274.18796.135508.105635@gargle.gargle.HOWL> you write:
Unfortunately, returning "getattr(self,method)(self,REQUEST)" requires the method to be DTML like and breaks if this is not the case.
Note that this particular problem can easily be cured by the standard if getattr(aq_base(ob), 'isDocTemp', 0) test (see CMFCore/PortalContent.py for an example).
But, it is probably best to ignore these difficulties and simply redirect always. Dieter
FWIW, I did file a detailed bug report on this issue in the (currently down) Zope collector... If we're talking about the same thing, I think the right thing to do is to change ActionProviderBase to not assume what it's calling is a DTML method. On Wed, 2003-10-22 at 06:25, Florent Guillaume wrote:
In article <16274.18796.135508.105635@gargle.gargle.HOWL> you write:
Unfortunately, returning "getattr(self,method)(self,REQUEST)" requires the method to be DTML like and breaks if this is not the case.
Note that this particular problem can easily be cured by the standard if getattr(aq_base(ob), 'isDocTemp', 0) test (see CMFCore/PortalContent.py for an example).
Florent
participants (4)
-
Chris McDonough -
Dieter Maurer -
Florent Guillaume -
Yuppie