search

plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
254 stars 191 forks source link

Breadcrumbs urls wrong if element with IHideFromBreadcrumbs in path #2935

Closed ksuess closed 5 years ago

ksuess commented 5 years ago

What I did:

Given a tree of folderish elements with one IHideFromBreadcrumbs element. folder1 > folder2> folder3 > page1 folder2 has IHideFromBreadcrumbs. So breadcrumbs for page1 look like folder1 > folder3 > page1

What I expect to happen:

Navigating via breadcrumbs up one or more levels.

What actually happened:

Navigating up one level to folder3 fails, because url of folder3 is wrong: /folder1/folder3 url folder1/folder2/folder3 would be correct.

What version of Plone/ Addons I am using:

Plone 5.1.5

my idea

This line overwrites the url: https://github.com/plone/Products.CMFPlone/blob/a43b250126980b2f5e2a011a578ca7f0d783b600/Products/CMFPlone/browser/navigation.py#L244 Commenting out fixes the problem and returns correct urls. But I think there is a reason this line of code was placed here. Someone any hint why?

petschki commented 5 years ago

ups ... that code is in master branch too and breadcrumbs seems to be untestet in general. Do you have the same issue in plone5.2 ?

ksuess commented 5 years ago

Same situation on Plone 5.2 master branch. Wrote a test which fails. With the line mentioned before removed, the test is OK.

def testBreadcrumbsFilterByInterface2(self):
    # Test url of subfolder of hidden folder.
    self.portal.folder1.invokeFactory('Folder', 'subfolder11')
    directlyProvides(self.portal.folder1.subfolder11, IHideFromBreadcrumbs)
    self.portal.folder1.subfolder11.invokeFactory('Folder', 'subfolder111')
    self.portal.folder1.subfolder11.subfolder111.invokeFactory('Document', 'doc1111')
    doc1111 = self.portal.folder1.subfolder11.subfolder111.doc1111
    view = self.view_class(doc1111, self.request)
    newcrumbs = view.breadcrumbs()
    self.assertEqual(
        newcrumbs[-2]['absolute_url'],
        'http://nohost/plone/folder1/subfolder11/subfolder111'
        )
petschki commented 5 years ago

I‘d make a PR with this an run the Jenkins Jobs.

ksuess commented 5 years ago

resolved for Plone 5.1 and Plone 5.2