Personal tools
You are here: Home Products Plone Roadmap #234: Standardizing our use of INavigationRoot
Document Actions

#234: Standardizing our use of INavigationRoot

Contents
  1. Motivation
  2. Proposal
  3. Implementation
  4. Risks
by Calvin Hendryx-Parker last modified November 6, 2008 - 00:22
Plone has the INavigationRoot marker interface which supports rooting your tabs, navigation, breadcrumbs, sitemaps and searches to a specific container inside of a portal which itself is a INavigationRoot. Some areas of this need to be cleaned up and standardized to follow a consistent convention.
Proposed by
Calvin Hendryx-Parker
Proposal type
Architecture
Assigned to release
State
in-progress

Motivation

Currently the INavigationRoot is used in quite a few areas. The main use is in NavtreeQueryBuilder and the SitemapNavtreeStrategy which are used by CatalogNavigationTree and CatalogNavigationTabs. You can also find it in the PhysicalNavigationBreadcrumbs which is the class for the breadcrumbs_view.

It appears that the intention of the live search and the global search box is to use getNavigationRoot() to constrain the search to the INavigationRoot, but the action of the form isn't obeying the INavigationRoot and always posts to the portal absolute_url instead. If you change the URL it posts to, the search boxes will work as expected because of the following:

  • In CMFPlone/skins/plone_scripts/livesearch_reply.py:70 getNavigationRoot() does get called, but since the URL that the livesearch posts to is the absolute_url of the portal the root isn't correct even if the search is done from a folder that does have the INavigationRoot marker interface.
  • CMFPlone/skins/plone_scripts/queryCatalog.py:72 does the same thing, but doesn't work from the global search box or search viewlet due to the action url of the form. If you call /Plone/mynavroot/search it will do the right thing since it will post back to mynavroot and getNavigationRoot() will find the mynavroot if it has the INavigationRoot marker interface on it.

The last place I have found this is in plone.app.redirector/plone/app/redirector/browser.py:72 where it currently works and constrains the search_for_similar method to just the INavigationRoot it finds.

The sitemap does do the right thing if it is called in the right context. Calling the sitemap_view anywhere in a container that has the INavigationRoot marker interface will give you a sitemap of just that root and not the whole portal. Again all that needs to be fixed here is the URL that is put in for the plone.site_actions so that it will write them rooted at the nearest INavigationRoot. Actually, all of the plone.site_actions should probably be written with the INavigationRoot in mind so they work correctly.

Proposal

We would need to fix at a minimum the search portlet and plone.searchbox viewlet which are both posting to the incorrect URLs. As it stands getNavigationRoot() never returns what would be expected if you are calling these from inside of a folder with the INavigationRoot marker on it.

At this point I don't think we need the ISearchRoot marker interface since it looks like this is really just a bug and not new functionality. The searches are supposed to be rooted at the INavigationRoot, but due to the form actions not being written correctly the code that does this never picks up the right root.

Implementation

  • Fix the site_url property of plone.app.layout.viewlets.common.ViewletBase to return the properly rooted url using the PortalState.navigation_root_url() instead of the portal absolute_url
  • Fix the plone.site_actions to generate their URLs based on navigation_root_url also

Risks

The only potential change in behavior would be when I fix plone.app.layout/plone/app/layout/viewlets/common.py to set the site_url property on the view to the INavigationRoot. If I do this then the search boxes will generate the URL for the form actions correctly, but the PersonalBarViewlet and DocumentBylineViewlet are using this property to write the url for the author page and dashboard links. I haven't seen any other code that is touching this so I think it is low risk at this point and would be good to change early in case people start depending on this.

do not change site_url

Posted by Wichert Akkerman at September 6, 2008 - 05:59
I'm -1 on changing the return value of site_url. Instead I would like to suggest to switch the actions to using a (possibly new) navigation_url method instead.

Suggestion

Posted by Christian Schneider at October 27, 2008 - 09:35
+1 on consistent behaviour for the INavigationRoot, but I would suggest splitting things up even further. Right now the INavigationRoot does a whole lot in one big thing, but I have encountered use cases where I want to have somewhat contained sections of a site that restrict searching and display portal tabs for one given section for example, but still have the breadcrumbs display every item up to the real site root. If every element had its own marker interface, i.e. IBreadcrumbRoot/ISearchRoot/IPortalTabsRoot/ISitemapRoot... things would be much more flexible. Creating the current INavigationRoot behaviour for sections where breadcrumbs, sitemap, search, portal tabs and navigation should be affected would just be a simple UI matter.

Framework team vote

Posted by Danny Bloemendaal at October 27, 2008 - 15:17
I refrain from voting on this plip. I don't have the knowledge to cast a proper vote.

3.3 Framework team vote

Posted by Martijn Pieters at October 27, 2008 - 15:49
I am -1 on this for the same reasons as Wichert; instead of altering the view to change site_url, add a navigation_root_url instead and alter views to use that attribute intead. If this proposal were changed in this way I'd vote +1.

Framework team vote (for Plone 3.3)

Posted by Andreas Zeidler at October 28, 2008 - 00:05
i agree with Wichert and Martijn, so -1 on changing `site_url`, but +1 when using a separate attribute.

ps: as for splitting up interfaces further another PLIP would be required — but this would be a nice one for 3.4, imho... ;)

Framework team vote (for Plone 3.3)

Posted by Andreas Zeidler at October 28, 2008 - 09:21
i'd like to update my vote to a definitive +1 after Calvin's response to our comments (see http://lists.plone.org/pipermail/framework-team/2008-October/002298.html)

framework team vote

Posted by Raphael Ritz at October 28, 2008 - 09:55
+1 on the updated version

3.3 Framework team vote

Posted by Martijn Pieters at October 28, 2008 - 09:59
+1 now that our concerns have been addressed.

Framework Team vote

Posted by Tom Lazar at October 28, 2008 - 10:18
+1 on the updated version, i.e. where we "don't change the return value of site_url, but instead change things over to use a navigation_root_url attribute instead"

For any issues with the web site functionality, please file a ticket.

Please consult the policy on plone.org content if you want your content published on this site.

Servers and hosting by