[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