On Mar 16, 2009, at 12:35 PM, Ross Patterson wrote:
David Glick <[email protected]> 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
[email protected]
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
_______________________________________________
Product-Developers mailing list
[email protected]
http://lists.plone.org/mailman/listinfo/product-developers
