Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29896

Navigation block: URL and file (resource) links broken

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            quen 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
            quen 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
            quen 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
            quen 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
            sbourget Stephen Bourget added a comment -

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

            Show
            sbourget Stephen Bourget added a comment - Is there a chance this fix can be included in an upcoming weekly release?
            Hide
            quen 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
            quen 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
            samhemelryk 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
            samhemelryk 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
            quen Sam Marshall added a comment -

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

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

            Integrated, thanks!

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

            This is working fine.

            Thanks

            Test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - This is working fine. Thanks Test passed.
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  12/Mar/12