silverstripe / silverstripe-cms

Silverstripe CMS - this is a module for Silverstripe Framework rather than a standalone app. Use https://github.com/silverstripe/silverstripe-installer/ to set this up.
http://silverstripe.org/
BSD 3-Clause "New" or "Revised" License
514 stars 333 forks source link

Duplicate breadcrumb labels when navigating site tree in ListView #2974

Closed maxime-rainville closed 3 months ago

maxime-rainville commented 3 months ago

Module version(s) affected

4.13.13 and 5.2.2

Description

When navigating the sitetree in list view, the "crumbs" in the "breadcumbs" get doubled up.

How to reproduce

  1. Access the CMS
  2. Create a top level page with the title of "First Level".
  3. Create a child page under "First Level" with the title of "Second Level".
  4. Create a child page under "Second Level" with the title of "Third Level".
  5. Create a child page under "Third Level" with the title of "Fourth Level".
  6. Switch to List View.
  7. Navigate to Third Level.

Expected: The breadcrumb looks like First Level / Second Level / Third Level Actual: The breadcrumb looks like First Level / Second Level / Third Level / First Level / Second Level / Third Level

Screenshot 2024-07-10 at 4 38 21 PM

Possible Solution

No response

Additional Context

No response

Validations

Pull request

maxime-rainville commented 3 months ago

I haven't look at the underlying cause. I'll see if there's a quick fix there tonight.

maxime-rainville commented 3 months ago

Don't have fix yet, but I think I narrowed the problem down a bit.

The problem is only apparent on PJAX request. After step 7, if you refresh, your breadcrumb will look like "Page / First Level / Second Level / Third Level". That's probably the correct end state.

Part of the problem is in the interrelation between CMSMain::Breadcrumbs and CMSPageController::Breadcrumbs

When doing a regular request, CMSMain will got through this if statement. updateBreadcrumbs will be called with an array containing only the a "Pages" link. https://github.com/silverstripe/silverstripe-cms/blob/7e10f298177b08b84723811467aa20baf04ad9e7/code/Controllers/CMSMain.php#L1058-L1070

When doing a PJAX request, CMSMain will skip the previous if statement and go through this logic instead.

https://github.com/silverstripe/silverstripe-cms/blob/7e10f298177b08b84723811467aa20baf04ad9e7/code/Controllers/CMSMain.php#L1072-L1086

This time updateBreadcrumbs will be called with the list of all the ancestors. Basically doubling the list.

maxime-rainville commented 3 months ago

I've update the expected behaviour to match have a "Pages" link in the breadcrumb.

image

maxime-rainville commented 3 months ago

Got a PR: https://github.com/silverstripe/silverstripe-cms/pull/2976

GuySartorelli commented 3 months ago

PR merged. This will be automatically tagged by GitHub Actions.