launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17525
[Merge] lp:~wgrant/launchpad/watermark-from-breadcrumbs into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/watermark-from-breadcrumbs into lp:launchpad.
Requested reviews:
William Grant (wgrant): code
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/watermark-from-breadcrumbs/+merge/241997
--
https://code.launchpad.net/~wgrant/launchpad/watermark-from-breadcrumbs/+merge/241997
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/app/browser/configure.zcml'
--- lib/lp/app/browser/configure.zcml 2014-05-14 11:05:22 +0000
+++ lib/lp/app/browser/configure.zcml 2014-11-17 18:03:10 +0000
@@ -72,7 +72,7 @@
/>
<browser:page
- for="Exception"
+ for="zope.browser.interfaces.ISystemErrorView"
name="+hierarchy"
class="lp.app.browser.launchpad.ExceptionHierarchy"
template="../templates/launchpad-hierarchy.pt"
@@ -905,14 +905,6 @@
name="required"
/>
- <!-- TALES watermark: namespace -->
- <adapter
- for="*"
- provides="zope.traversing.interfaces.IPathAdapter"
- factory="lp.app.browser.watermark.WatermarkTalesAdapter"
- name="watermark"
- />
-
<!-- TALES fmt: namespace for strings -->
<adapter
for="basestring"
=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py 2014-11-16 09:33:01 +0000
+++ lib/lp/app/browser/launchpad.py 2014-11-17 18:03:10 +0000
@@ -54,7 +54,11 @@
TextLine,
)
from zope.security.interfaces import Unauthorized
-from zope.traversing.interfaces import ITraversable
+from zope.security.proxy import removeSecurityProxy
+from zope.traversing.interfaces import (
+ IPathAdapter,
+ ITraversable,
+ )
from lp import _
from lp.answers.interfaces.questioncollection import IQuestionSet
@@ -75,7 +79,11 @@
NotFoundError,
POSTToNonCanonicalURL,
)
-from lp.app.interfaces.headings import IMajorHeadingView
+from lp.app.interfaces.headings import (
+ IEditableContextTitle,
+ IMajorHeadingView,
+ IRootContext,
+ )
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.app.interfaces.services import IServiceFactory
from lp.app.widgets.project import ProjectScopeWidget
@@ -131,6 +139,7 @@
)
from lp.services.webapp.authorization import check_permission
from lp.services.webapp.breadcrumb import Breadcrumb
+from lp.services.webapp.escaping import structured
from lp.services.webapp.interfaces import (
IBreadcrumb,
ICanonicalUrlData,
@@ -309,7 +318,9 @@
@property
def _naked_context_view(self):
"""Return the unproxied view for the context of the hierarchy."""
- from zope.security.proxy import removeSecurityProxy
+ # XXX wgrant 2014-11-16: Should just be able to use self.context
+ # here, but some views end up delegating to things that don't
+ # have __name__ or __launchpad_facetname__.
if len(self.request.traversed_objects) > 0:
return removeSecurityProxy(self.request.traversed_objects[-1])
else:
@@ -363,6 +374,55 @@
self._naked_context_view)
return len(self.items) > 1 and not has_major_heading
+ @property
+ def heading_breadcrumb(self):
+ try:
+ return (
+ crumb for crumb in self.items
+ if IRootContext.providedBy(crumb.context)).next()
+ except StopIteration:
+ return None
+
+ def heading(self):
+ """Return the heading text for the page.
+
+ If the view provides `IEditableContextTitle` then the top heading is
+ rendered from the view's `title_edit_widget` and is generally
+ editable.
+
+ Otherwise, if the context provides `IHeadingContext` then we return an
+ H1, else an H2.
+ """
+ # Check the view; is the title editable?
+ if IEditableContextTitle.providedBy(self.context):
+ return self.context.title_edit_widget()
+ # The title is static, but only the context's index view gets an H1.
+ heading = 'h1' if IMajorHeadingView.providedBy(self.context) else 'h2'
+ # If there is actually no root context, then it's a top-level
+ # context-less page so Launchpad.net is shown as the branding.
+ if self.heading_breadcrumb:
+ title = self.heading_breadcrumb.detail
+ else:
+ title = 'Launchpad.net'
+ # For non-editable titles, generate the static heading.
+ return structured(
+ "<%(heading)s>%(title)s</%(heading)s>",
+ heading=heading, title=title).escapedtext
+
+ def logo(self):
+ """Return the logo image for the top header breadcrumb's context."""
+ logo_context = (
+ self.heading_breadcrumb.context if self.heading_breadcrumb
+ else None)
+ adapter = queryAdapter(logo_context, IPathAdapter, 'image')
+ if logo_context != self.context.context and logo_context is not None:
+ return structured(
+ '<a href="%s">%s</a>',
+ canonical_url(logo_context, rootsite='mainsite'),
+ structured(adapter.logo())).escapedtext
+ else:
+ return adapter.logo()
+
class ExceptionHierarchy(Hierarchy):
=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py 2014-08-29 01:34:04 +0000
+++ lib/lp/app/browser/tales.py 2014-11-17 18:03:10 +0000
@@ -692,8 +692,7 @@
ROOT_TITLE = 'Launchpad'
view = self._context
request = get_current_browser_request()
- hierarchy_view = getMultiAdapter(
- (view.context, request), name='+hierarchy')
+ hierarchy_view = getMultiAdapter((view, request), name='+hierarchy')
if (isinstance(view, SystemErrorView) or
hierarchy_view is None or
not hierarchy_view.display_breadcrumbs):
=== modified file 'lib/lp/app/browser/tests/watermark.txt'
--- lib/lp/app/browser/tests/watermark.txt 2012-12-11 05:41:50 +0000
+++ lib/lp/app/browser/tests/watermark.txt 2014-11-17 18:03:10 +0000
@@ -1,57 +1,65 @@
-===============================
-The "watermark" TALES formatter
-===============================
+=======================
+The "watermark" heading
+=======================
-The watermark formatter is used to get the image and heading used on all the
-main Launchpad pages. The image and heading are determined by the
+The watermark is the image and heading used on all the main Launchpad
+pages. The image and heading are determined by Hierarchy from the
IRootContext for the context object.
Watermark headings
==================
-The `watermark:heading` is used when you want a heading for the nearest object
-that implements IRootContext.
+Hierarchy.heading() is used when you want a heading for the nearest
+object that implements IRootContext.
- >>> from lp.testing import test_tales
- >>> class view:
+ >>> from lp.app.browser.launchpad import Hierarchy
+ >>> from lp.services.webapp.servers import LaunchpadTestRequest
+ >>> class TrivialView:
+ ... __name__ = '+trivial'
... def __init__(self, context):
... self.context = context
+ >>> def get_hierarchy(obj, viewcls=TrivialView):
+ ... req = LaunchpadTestRequest()
+ ... view = viewcls(obj)
+ ... req.traversed_objects.append(view.context)
+ ... req.traversed_objects.append(view)
+ ... return Hierarchy(view, req)
Products directly implement IRootContext.
>>> widget = factory.makeProduct(title='Widget')
- >>> print test_tales('view/watermark:heading', view=view(widget))
+ >>> print get_hierarchy(widget).heading()
<h...>Widget</h...>
A series of the product still show the product watermark.
>>> dev_focus = widget.development_focus
- >>> print test_tales('view/watermark:heading', view=view(dev_focus))
+ >>> print get_hierarchy(dev_focus).heading()
<h...>Widget</h...>
ProjectGroups also directly implement IRootContext ...
>>> kde = factory.makeProject(title='KDE')
- >>> print test_tales('view/watermark:heading', view=view(kde))
+ >>> print get_hierarchy(kde).heading()
<h...>KDE</h...>
... as do distributions ...
>>> mint = factory.makeDistribution(title='Mint Linux')
- >>> print test_tales('view/watermark:heading', view=view(mint))
+ >>> print get_hierarchy(mint).heading()
<h...>Mint Linux</h...>
... and people ...
>>> eric = factory.makePerson(displayname="Eric the Viking")
- >>> print test_tales('view/watermark:heading', view=view(eric))
+ >>> print get_hierarchy(eric).heading()
<h...>Eric the Viking</h...>
... and sprints.
>>> sprint = factory.makeSprint(title="Launchpad Epic")
- >>> print test_tales('view/watermark:heading', view=view(sprint))
+ >>> print get_hierarchy(sprint).heading()
<h...>Launchpad Epic</h...>
If there is no root context defined for the object, then the heading is
@@ -59,14 +67,14 @@
Launchpad.net).
>>> machine = factory.makeCodeImportMachine()
- >>> print test_tales('view/watermark:heading', view=view(machine))
+ >>> print get_hierarchy(machine).heading()
<h...>Launchpad.net</h...>
Any HTML in the context title will be escaped to avoid XSS vulnerabilities.
>>> person = factory.makePerson(
... displayname="Fubar<br/><script>alert('XSS')</script>")
- >>> print test_tales('view/watermark:heading', view=view(person))
+ >>> print get_hierarchy(person).heading()
<h...>Fubar<br/><script>alert('XSS')</script></h...>
@@ -75,15 +83,15 @@
The image for the watermark is determined effectively by context/image:logo.
- >>> print test_tales('view/watermark:logo', view=view(dev_focus))
+ >>> print get_hierarchy(dev_focus).logo()
<a href="..."><img ... src="/@@/product-logo" /></a>
- >>> print test_tales('view/watermark:logo', view=view(eric))
+ >>> print get_hierarchy(eric).logo()
<img ... src="/@@/person-logo" />
If there is no root context, the Launchpad logo is shown.
- >>> print test_tales('view/watermark:logo', view=view(machine))
+ >>> print get_hierarchy(machine).logo()
<img ... src="/@@/launchpad-logo" />
@@ -100,7 +108,7 @@
is not the index page of the context. In this case the heading is rendered in
H2.
- >>> print test_tales('view/watermark:heading', view=view(widget))
+ >>> print get_hierarchy(widget).heading()
<h2>Widget</h2>
If the view class implements IMajorHeadingView, then this is the index page
@@ -108,13 +116,9 @@
>>> from zope.interface import implements
>>> from lp.app.interfaces.headings import IMajorHeadingView
-
- >>> class view:
+ >>> class HeadingView(TrivialView):
... implements(IMajorHeadingView)
- ... def __init__(self, context):
- ... self.context = context
-
- >>> print test_tales('view/watermark:heading', view=view(widget))
+ >>> print get_hierarchy(widget, viewcls=HeadingView).heading()
<h1>Widget</h1>
@@ -127,12 +131,11 @@
appropriate H tag.
>>> from lp.app.interfaces.headings import IEditableContextTitle
- >>> class view:
+ >>> class EditableView(TrivialView):
... implements(IEditableContextTitle)
... def __init__(self, context):
... self.context = context
... def title_edit_widget(self):
... return '<h0>EDIT ME</h0>'
-
- >>> print test_tales('view/watermark:heading', view=view(widget))
+ >>> print get_hierarchy(widget, viewcls=EditableView).heading()
<h0>EDIT ME</h0>
=== removed file 'lib/lp/app/browser/watermark.py'
--- lib/lp/app/browser/watermark.py 2012-12-11 00:32:46 +0000
+++ lib/lp/app/browser/watermark.py 1970-01-01 00:00:00 +0000
@@ -1,90 +0,0 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""The watermark TALES path adapter."""
-
-__metaclass__ = type
-__all__ = [
- 'WatermarkTalesAdapter',
- ]
-
-
-from zope.component import queryAdapter
-from zope.interface import implements
-from zope.traversing.interfaces import (
- IPathAdapter,
- ITraversable,
- TraversalError,
- )
-
-from lp.app.interfaces.headings import (
- IEditableContextTitle,
- IMajorHeadingView,
- IRootContext,
- )
-from lp.services.webapp.canonicalurl import nearest_provides_or_adapted
-from lp.services.webapp.escaping import structured
-from lp.services.webapp.publisher import canonical_url
-
-
-class WatermarkTalesAdapter:
- """Adapter for any object to get the watermark heading and image."""
-
- implements(ITraversable)
-
- def __init__(self, view):
- self._view = view
- self._context = view.context
-
- @property
- def root_context(self):
- return nearest_provides_or_adapted(self._context, IRootContext)
-
- def heading(self):
- """Return the heading text for the page.
-
- If the view provides `IEditableContextTitle` then the top heading is
- rendered from the view's `title_edit_widget` and is generally
- editable.
-
- Otherwise, if the context provides `IRootContext` then we return an
- H1, else an H2.
- """
- # Check the view; is the title editable?
- if IEditableContextTitle.providedBy(self._view):
- return self._view.title_edit_widget()
- # The title is static, but only the context's index view gets an H1.
- if IMajorHeadingView.providedBy(self._view):
- heading = structured('h1')
- else:
- heading = structured('h2')
- # If there is actually no root context, then it's a top-level
- # context-less page so Launchpad.net is shown as the branding.
- if self.root_context is None:
- title = 'Launchpad.net'
- else:
- title = self.root_context.title
- # For non-editable titles, generate the static heading.
- return structured(
- "<%(heading)s>%(title)s</%(heading)s>",
- heading=heading,
- title=title).escapedtext
-
- def logo(self):
- """Return the logo image for the root context."""
- adapter = queryAdapter(self.root_context, IPathAdapter, 'image')
- if (self.root_context != self._context
- and self.root_context is not None):
- return '<a href="%s">%s</a>' % (
- canonical_url(self.root_context, rootsite='mainsite'),
- adapter.logo())
- else:
- return adapter.logo()
-
- def traverse(self, name, furtherPath):
- if name == "heading":
- return self.heading()
- elif name == "logo":
- return self.logo()
- else:
- raise TraversalError(name)
=== modified file 'lib/lp/app/templates/base-layout.pt'
--- lib/lp/app/templates/base-layout.pt 2012-11-12 15:49:29 +0000
+++ lib/lp/app/templates/base-layout.pt 2014-11-17 18:03:10 +0000
@@ -93,10 +93,10 @@
<div id="watermark" class="watermark-apps-portlet"
tal:condition="view/macro:has-watermark">
<div>
- <span tal:replace="structure view/watermark:logo"></span>
+ <span tal:replace="structure view/@@+hierarchy/logo"></span>
</div>
<div class="wide">
- <h2 tal:replace="structure view/watermark:heading">
+ <h2 tal:replace="structure view/@@+hierarchy/heading">
Celso Providelo
</h2>
<metal:heading_nav
@@ -119,7 +119,7 @@
metal:define-slot="heading"
>Page Label
</h1>
- <tal:breadcrumbs replace="structure context/@@+hierarchy">
+ <tal:breadcrumbs replace="structure view/@@+hierarchy">
ProjectName > Branches > Merge Proposals > fix-for-navigation
</tal:breadcrumbs>
<div id="registration" class="registering"
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2014-11-16 09:17:22 +0000
+++ lib/lp/code/browser/branch.py 2014-11-17 18:03:10 +0000
@@ -189,11 +189,6 @@
return self.context.target.components[-1]
-def branch_root_context(branch):
- """Return the IRootContext for the branch."""
- return branch.target.components[0]
-
-
class BranchNavigation(Navigation):
usedfor = IBranch
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2014-11-16 09:49:59 +0000
+++ lib/lp/code/browser/configure.zcml 2014-11-17 18:03:10 +0000
@@ -19,11 +19,6 @@
PersonRevisionFeed ProductRevisionFeed ProjectRevisionFeed"
/>
- <adapter
- provides="lp.app.interfaces.headings.IRootContext"
- for="lp.code.interfaces.branch.IBranch"
- factory="lp.code.browser.branch.branch_root_context"/>
-
<facet facet="branches">
<browser:defaultView
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2013-03-09 09:15:47 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2014-11-17 18:03:10 +0000
@@ -17,7 +17,6 @@
from zope.security.proxy import removeSecurityProxy
from lp.app.enums import InformationType
-from lp.app.interfaces.headings import IRootContext
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.app.interfaces.services import IService
from lp.bugs.interfaces.bugtask import (
@@ -893,30 +892,6 @@
self.assertEqual([], view.dependent_branches)
-class TestBranchRootContext(TestCaseWithFactory):
- """Test the adaptation of IBranch to IRootContext."""
-
- layer = DatabaseFunctionalLayer
-
- def test_personal_branch(self):
- # The root context of a personal branch is the person.
- branch = self.factory.makePersonalBranch()
- root_context = IRootContext(branch)
- self.assertEqual(branch.owner, root_context)
-
- def test_package_branch(self):
- # The root context of a package branch is the distribution.
- branch = self.factory.makePackageBranch()
- root_context = IRootContext(branch)
- self.assertEqual(branch.distroseries.distribution, root_context)
-
- def test_product_branch(self):
- # The root context of a product branch is the product.
- branch = self.factory.makeProductBranch()
- root_context = IRootContext(branch)
- self.assertEqual(branch.product, root_context)
-
-
class TestBranchEditView(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py 2014-11-16 09:34:58 +0000
+++ lib/lp/registry/browser/pillar.py 2014-11-17 18:03:10 +0000
@@ -62,7 +62,10 @@
BatchNavigator,
StormRangeFactory,
)
-from lp.services.webapp.breadcrumb import Breadcrumb
+from lp.services.webapp.breadcrumb import (
+ Breadcrumb,
+ DisplaynameBreadcrumb,
+ )
from lp.services.webapp.menu import (
ApplicationMenu,
enabled_with_permission,
@@ -77,6 +80,14 @@
)
+class PillarBreadcrumb(DisplaynameBreadcrumb):
+ """Breadcrumb that uses the displayname or title as appropriate."""
+
+ @property
+ def detail(self):
+ return self.context.title
+
+
class PillarPersonBreadcrumb(Breadcrumb):
"""Builds a breadcrumb for an `IPillarPerson`."""
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2014-07-31 00:23:58 +0000
+++ lib/lp/registry/configure.zcml 2014-11-17 18:03:10 +0000
@@ -457,7 +457,7 @@
<adapter
provides="lp.services.webapp.interfaces.IBreadcrumb"
for="lp.registry.interfaces.projectgroup.IProjectGroup"
- factory="lp.services.webapp.breadcrumb.DisplaynameBreadcrumb"
+ factory="lp.registry.browser.pillar.PillarBreadcrumb"
permission="zope.Public"/>
<adapter
provides="lp.services.webapp.interfaces.IBreadcrumb"
@@ -1499,7 +1499,7 @@
<adapter
provides="lp.services.webapp.interfaces.IBreadcrumb"
for="lp.registry.interfaces.product.IProduct"
- factory="lp.services.webapp.breadcrumb.DisplaynameBreadcrumb"
+ factory="lp.registry.browser.pillar.PillarBreadcrumb"
permission="zope.Public"/>
<adapter
provides="lp.services.webapp.interfaces.IBreadcrumb"
@@ -1808,7 +1808,7 @@
<adapter
provides="lp.services.webapp.interfaces.IBreadcrumb"
for="lp.registry.interfaces.distribution.IDistribution"
- factory="lp.services.webapp.breadcrumb.DisplaynameBreadcrumb"
+ factory="lp.registry.browser.pillar.PillarBreadcrumb"
permission="zope.Public"/>
<adapter
provides="lp.services.webapp.interfaces.IBreadcrumb"
=== modified file 'lib/lp/services/webapp/tests/test_breadcrumbs.py'
--- lib/lp/services/webapp/tests/test_breadcrumbs.py 2014-02-25 08:51:21 +0000
+++ lib/lp/services/webapp/tests/test_breadcrumbs.py 2014-11-17 18:03:10 +0000
@@ -118,10 +118,11 @@
page_title = Message(
'${name} test', mapping={'name': 'breadcrumb'})
__name__ = 'test-page'
+ context = self.product
test_view = TestView()
request = LaunchpadTestRequest()
request.traversed_objects = [self.product, test_view]
- hierarchy_view = Hierarchy(self.product, request)
+ hierarchy_view = Hierarchy(test_view, request)
breadcrumb = hierarchy_view.makeBreadcrumbForRequestedPage()
self.assertEquals(breadcrumb.text, 'breadcrumb test')
=== modified file 'lib/lp/testing/breadcrumbs.py'
--- lib/lp/testing/breadcrumbs.py 2012-01-01 02:58:52 +0000
+++ lib/lp/testing/breadcrumbs.py 2014-11-17 18:03:10 +0000
@@ -3,6 +3,8 @@
__metaclass__ = type
+from zope.security.proxy import removeSecurityProxy
+
from lp.services.webapp.publisher import canonical_url
from lp.testing import TestCaseWithFactory
from lp.testing.layers import DatabaseFunctionalLayer
@@ -48,5 +50,8 @@
def getBreadcrumbsForUrl(self, url):
obj, view, request = test_traverse(url)
- view = create_initialized_view(obj, '+hierarchy', request=request)
- return view.items
+ # Sometimes test_traverse returns the __call__, while the template
+ # always has access to the instance.
+ view = getattr(removeSecurityProxy(view), 'im_self', view)
+ hier = create_initialized_view(view, '+hierarchy', request=request)
+ return hier.items
References
