Re: review for fixes of bugs due to acquisition black magic

Am 07.11.2013 19:14, schrieb David Glick (Plone):
> On 11/7/13, 3:42 AM, Maurits van Rees wrote:
>> Hi Godefroid,
>>
>> Godefroid Chapelle schreef op 06-11-13 18:09:
>>> Hi all,
>>>
>>> I committed fixes in 4.3.x branch for two nasty acquisition bugs.
>>>
>>> I'd like to get some feedback :
>>>
>>> -
>>> https://github.com/plone/Products.CMFPlone/commit/e20ea907d2854ee7ad41e9953b64889ee7431b49
>>>
>>>
>>> is a fix for https://dev.plone.org/ticket/13793
>>>
>>> but it might break some sites that have wrongly built URLS, iow URLS
>>> where content is currently made accessible through acquisition.
>> This has the potential to break sites that rely on 'wrongly built urls'.
>>    One example is when the main_template has '<img src="header.jpg" />'
>> to show the header.jpg in the context.  This is a simple way to have
>> different header images for different parts of your site.  There are
>> better ways, especially if you consider the effect this has on caching,
>> but I think this does happen.
>>
>> You give an example in the ticket: Joe deletes root/department/news and
>> Jane has this in a bookmark, visits it and gets served the content of
>> root/news under the url root/department/news.  What happens if Jane
>> visits root/department/news/some_template?  Does your event handler
>> raise a 404 then?
>>
>> Oh, sudden alternative idea: do not raise a 404 but simply redirect to
>> the correct url.  That may still have unwanted side effects, but it
>> would be less, I think.
>>
>> For example it may redirect to a lower level url that is not available.
>>    The root/department folder may be a navigation root and may be
>> available at department.example.org.  If Jane visits
>> department.example.org/news I cannot think of a correct url to redirect to.
>>
>> It may be totally intended that she sees the content that is in reality
>> located at root/news.  This is an argument against redirecting, but also
>> against raising a 404.
>>
>> Possibly, the code could check whether the new url is within the same
>> navigation root and only redirect then.  If it is outside the navigation
>> root, it may be best to keep the current behavior and just serve the
>> acquired content.
> 
> I like trying to address this problem, but I am also worried about 
> breaking existing sites that inadvertently rely on Acquisition in their 
> links. Perhaps we should only make the change for Plone 5.

This commit also broke the 4.3 build:

http://jenkins.plone.org/job/plone-4.3-python-2.7/1585/

This has not been fixed yet, therefore the commit should be reverted
immediately in any case.

The testbot mailinglist seems to be broken right now, Ramon is working
on a fix.

Timo

------------------------------------------------------------------------------
November Webinars for C, C++, Fortran Developers
Accelerate application performance with scalable programming models. Explore
techniques for threading, error checking, porting, and tuning. Get the most 
from the latest Intel processors and coprocessors. See abstracts and register
http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk