[Product-Developers] Re: making collective.flowplayer less invasive

David Glick davidglick at onenw.org
Mon Mar 16 19:58:04 UTC 2009


On Mar 16, 2009, at 12:35 PM, Ross Patterson wrote:

> David Glick <davidglick at onenw.org> writes:
>
>> I've just been reviewing the collective.flowplayer code as part of  
>> the
>> process for approving it for use on our Plone sites.  It looks great,
>> with one exception: we host many Plone sites in one Zope instance,  
>> and
>> want to selectively install collective.flowplayer to only some of
>> those sites.  Based on inspection of the code, it looks like there  
>> are
>> two ways in which collective.flowplayer will pollute the other sites:
>>
>> 1. It registers various browser views without specifying a browser
>> layer.
>
> As long as this doesn't change the "install it and it just works"  
> story
> and it doesn't require integrators who want to override those views to
> do something special, I'm all for doing this if it's better practice.
> Can you point me to how this is done for something like the
> folder_contents view in Plone?  Would it change *anything* for
> integrators overriding the views?

It shouldn't change the install experience or require anything special  
for overriding the view.  If integrators are overriding the views they  
should be doing so by copying the view registration and specifying a  
theme-specific browser layers (from plone.theme), which takes  
precedence over the additive browser layers from plone.browserlayer.

Overriding folder_contents is different because it's a skin layer item  
rather than a Zope 3 view.  So you override using a new CMF skin layer  
with higher precedence.  This isn't relevant to collective.flowplayer  
though.

>> 2. It subtypes objects in response to object events without checking
>> to make sure that the object is within a site that has
>> collective.flowplayer installed.
>
> I'm not sure what you're talking about here.  Can you please specify  
> all
> the places this happens?

configure.zcml registers the following event handlers:

    <subscriber
         for="Products.ATContentTypes.interface.IATFile
              Products.Archetypes.interfaces.IObjectInitializedEvent"
         handler=".events.ChangeFileView"
         />
     <subscriber
         for="Products.ATContentTypes.interface.IATFile
              Products.Archetypes.interfaces.IObjectEditedEvent"
         handler=".events.ChangeFileView"
         />

     <subscriber
         for="Products.ATContentTypes.interface.IATLink
              Products.Archetypes.interfaces.IObjectInitializedEvent"
         handler=".events.ChangeLinkView"
         />
     <subscriber
         for="Products.ATContentTypes.interface.IATLink
              Products.Archetypes.interfaces.IObjectEditedEvent"
         handler=".events.ChangeLinkView"
         />

These are global registrations.  But the ChangeXXXView classes don't  
do any checks to make sure that the object is in a site with  
collective.flowplayer installed, so they will run even for objects in  
other sites.

The simplest way to fix this is to add a check like:

from zope.app.component.hooks import getSite
request = getSite().REQUEST
if not IMyProductsBrowserLayer.providedBy(request):
     pass

at the top of each event handler.

(I think it is possible to register local event handlers in the ZCA,  
which would be more elegant, but there's no GS support for this at  
present.)

>> Both of these should be pretty easy to fix...Can we do that?
>
> Once discussion is finished, you're welcome to commit the changes to
> trunk.  :)
>
> Ross

David Glick
Web Developer
ONE/Northwest

New tools and strategies for engaging people in protecting the  
environment

http://www.onenw.org
davidglick at onenw.org
work: (206) 286-1235 x32
mobile: (206) 679-3833

Subscribe to ONEList, our email newsletter!
Practical advice for effective online engagement
http://www.onenw.org/full_signup








More information about the Product-Developers mailing list