Moodle
  1. Moodle
  2. MDL-32393

Duplicate Page Name in Breadcrumbs for Page Set to Use Popup

    Details

    • Testing Instructions:
      Hide
      1. Enable the popup display option for the page module.
      2. Create a new page activity with the display option set to popup.
      3. Click Save and display
      4. Check that the navigation bar only shows the activity name once.
      5. Browse to the course and click on the page link in the main content.
      6. Ensure you get a popup.
      Show
      Enable the popup display option for the page module. Create a new page activity with the display option set to popup. Click Save and display Check that the navigation bar only shows the activity name once. Browse to the course and click on the page link in the main content. Ensure you get a popup.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      wip-MDL-32393-m23
    • Rank:
      39251

      Description

      When a Page resource is set to open in a popup window and you either edit that resource or click it from the Recent Activity block, the breadcrumbs on the resulting page list the page name twice. For example, if the page name is "Page" the breadcrumb entry for the page shows up as "PagePage". I have reproduced this in 2.1.3 and the latest 2.3. See attached screenshot from the latest 2.3, where it also includes an icon in between the double names. My 2.1.3 does not have the icon but it is using a custom theme whereas my 2.3 is not.

      Steps to reproduce:

      • Create a new Page in a course and set the page options to display in a popup. Save.
      • Click the edit icon next to the page name or click the page name in the Recent Activity block.
      • Notice the double name in the breadcrumbs.

      It does not double the page name when the page display is set to "Open" rather than "In a popup."

      The line of code that is duplicating the name is in render_navigation_node() within lib/outputrenderers.php, line 2514 in the current 2.3:

      2514             $link->text = $content.$link->text; // add help icon
      

      $content already has the name and then the name is appended to it, duplicating it. I don't know this part of code well enough to know if the function itself is wrong or if what's being fed into the function is wrong.

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Thanks for the report Chris, I've looked into it now and you were spot on in noting where the issue is arising from.
          I've just put a solution up for peer-review now that involves replacing the link content with the navigation nodes generated content.
          This appears the be the correct option, removing the line leads to the icon appearing on the navigation bar which is inconsistent, and we can't remove the icon from the action_link as it is required for the AJAX loading of the navigation.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks for the report Chris, I've looked into it now and you were spot on in noting where the issue is arising from. I've just put a solution up for peer-review now that involves replacing the link content with the navigation nodes generated content. This appears the be the correct option, removing the line leads to the icon appearing on the navigation bar which is inconsistent, and we can't remove the icon from the action_link as it is required for the AJAX loading of the navigation. Cheers Sam
          Hide
          Chris Follin added a comment -

          Thanks, Sam. I tested your fix and it resolves the problem.

          Show
          Chris Follin added a comment - Thanks, Sam. I tested your fix and it resolves the problem.
          Hide
          Sam Hemelryk added a comment -

          Awesome thanks for testing that Chris, I'm putting this up for integration now

          Show
          Sam Hemelryk added a comment - Awesome thanks for testing that Chris, I'm putting this up for integration now
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          Hi Sam,

          Don't see a wip-MDL-32393-m23 branch..

          Show
          Dan Poltawski added a comment - Hi Sam, Don't see a wip- MDL-32393 -m23 branch..
          Hide
          Sam Hemelryk added a comment -

          Sorry Dan, I must've cleaned it up accidentally before this weeks integration.
          I've pushed it back up now.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Sorry Dan, I must've cleaned it up accidentally before this weeks integration. I've pushed it back up now. Cheers Sam
          Hide
          Dan Poltawski added a comment -

          Thanks Sam/Chris,

          i've integrated this now

          Show
          Dan Poltawski added a comment - Thanks Sam/Chris, i've integrated this now
          Hide
          Adrian Greeve added a comment -

          Checked in master (pre-patch) and then tested in 2.1, 2.2 and master (post-patch) Everything works as expected.
          Thanks.

          Show
          Adrian Greeve added a comment - Checked in master (pre-patch) and then tested in 2.1, 2.2 and master (post-patch) Everything works as expected. Thanks.
          Hide
          Dan Poltawski added a comment -

          Bonza mate!

          Your changes have made it into the Moodle release! Its time to celebrate! Put a shrimp on the barbie and grab a stubby.

          Hooroo

          Show
          Dan Poltawski added a comment - Bonza mate! Your changes have made it into the Moodle release! Its time to celebrate! Put a shrimp on the barbie and grab a stubby. Hooroo

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: