Moodle
  1. Moodle
  2. MDL-29896

Navigation block: URL and file (resource) links broken

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3
    • Fix Version/s: 2.2.2
    • Component/s: Navigation
    • Labels:
      None
    • Testing Instructions:
      Hide

      1. In a course week, click Add a resource / URL.
      2. Set name 'Google', description 'x', external URL 'http://www.google.com/', display 'In pop-up'. (Leave other options default.)
      3. Click 'Save and return to course'.
      4. In a course week, click Add a resource / File.
      5. Set name 'File', description 'x', click 'Add...' and select any PDF file. Set display to 'In pop-up'. Leave other options default.
      6. Click 'Save and return to course'.
      7. Where they appear within the course week, click both links.

      • Verify that they still work (to launch Google, and the PDF file, in popup windows.)
        8. In the Navigation block, expand the week in question.
      • Verify that both items appear as normal clickable links.
        9. Click on each link in the navigation block.
      • Verify that both links still work.
      Show
      1. In a course week, click Add a resource / URL. 2. Set name 'Google', description 'x', external URL 'http://www.google.com/', display 'In pop-up'. (Leave other options default.) 3. Click 'Save and return to course'. 4. In a course week, click Add a resource / File. 5. Set name 'File', description 'x', click 'Add...' and select any PDF file. Set display to 'In pop-up'. Leave other options default. 6. Click 'Save and return to course'. 7. Where they appear within the course week, click both links. Verify that they still work (to launch Google, and the PDF file, in popup windows.) 8. In the Navigation block, expand the week in question. Verify that both items appear as normal clickable links. 9. Click on each link in the navigation block. Verify that both links still work.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-29896-master
    • Rank:
      19427

      Description

      On my Moodle 2.2 dev system, when you add a URL or file resource and you set it to open in a popup window, the link in the navigation block stops working altogether.

      This might possibly be related to changes I made a while back, so I'll have a look at it initially at least.

        Issue Links

          Activity

          Hide
          Sam Marshall added a comment -

          OK, I found out why this isn't working - it's because these nodes use an 'activity_link' object which is not supported in the JSON version.

          I have done a fix for this (see git url), but this does not address the issue that the 'onclick' event itself is not actually used (so it just goes to the intermediate page for the activity/link). Implementing that will be difficult because the way it works for non-JSON behaviour, it's created some function or other and given a reference to that, but unfortunately the function itself is not included in the JSON response...

          Anyway this code is at least an improvement from 'link totally broken' to 'link works but ignoring the popup option'. Passing to Sam Hemelryk, up to you if you want to put this in or improve it or whatever...

          Show
          Sam Marshall added a comment - OK, I found out why this isn't working - it's because these nodes use an 'activity_link' object which is not supported in the JSON version. I have done a fix for this (see git url), but this does not address the issue that the 'onclick' event itself is not actually used (so it just goes to the intermediate page for the activity/link). Implementing that will be difficult because the way it works for non-JSON behaviour, it's created some function or other and given a reference to that, but unfortunately the function itself is not included in the JSON response... Anyway this code is at least an improvement from 'link totally broken' to 'link works but ignoring the popup option'. Passing to Sam Hemelryk, up to you if you want to put this in or improve it or whatever...
          Hide
          Sam Marshall added a comment -

          note: I just checked in 2.1 and in 2.1, the link works but the onclick (popup) doesn't - same as after this fix.

          Show
          Sam Marshall added a comment - note: I just checked in 2.1 and in 2.1, the link works but the onclick (popup) doesn't - same as after this fix.
          Hide
          Stephen Bourget added a comment -

          Is there a chance this fix can be included in an upcoming weekly release?

          Show
          Stephen Bourget added a comment - Is there a chance this fix can be included in an upcoming weekly release?
          Hide
          Sam Marshall added a comment -

          Stephen: Requesting peer review for this fix (only). I had to assign the bug to me to do that.

          The other issues mentioned obviously remain.

          Show
          Sam Marshall added a comment - Stephen: Requesting peer review for this fix (only). I had to assign the bug to me to do that. The other issues mentioned obviously remain.
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          Sorry I totally missed this issue.
          Certainly it is an improvement over the current functionality and I think this should be put up for integration.
          Upon being integrated we should as well create a new issue to sort out the less critical task of the popup option.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, Sorry I totally missed this issue. Certainly it is an improvement over the current functionality and I think this should be put up for integration. Upon being integrated we should as well create a new issue to sort out the less critical task of the popup option. Cheers Sam
          Hide
          Sam Marshall added a comment -

          Thanks Sam, I have rebased this change and am submitting it for integration.

          Show
          Sam Marshall added a comment - Thanks Sam, I have rebased this change and am submitting it for integration.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Rossiani Wijaya added a comment -

          This is working fine.

          Thanks

          Test passed.

          Show
          Rossiani Wijaya added a comment - This is working fine. Thanks Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many.

          Nah, joking, many thanks! Closing this a fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many. Nah, joking, many thanks! Closing this a fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: