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

Accessing file resources without frame fails to create log entry

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.8, 1.9.9, 1.9.13
    • Fix Version/s: 1.9.15
    • Component/s: Resource
    • Labels:
    • Testing Instructions:
      Hide

      1. Create a file resource that links to an uploaded HTML file
      2. Name it 'Yes, without frame'
      3. Click 'Advanced' under 'Window' section
      4. Change 'Show Navigation' to 'Yes, without frame'
      5. Save and view resource a couple of times
      6. View logs and note that no log entry has been created
      7. Apply patch, repeat steps 1-5, view log and find entry.

      Show
      1. Create a file resource that links to an uploaded HTML file 2. Name it 'Yes, without frame' 3. Click 'Advanced' under 'Window' section 4. Change 'Show Navigation' to 'Yes, without frame' 5. Save and view resource a couple of times 6. View logs and note that no log entry has been created 7. Apply patch, repeat steps 1-5, view log and find entry.
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Pull from Repository:

      Description

      To reproduce, create a file resource (a pdf, for example) and set "Show navigation" to "Yes, without frame". Then access the resource. Look at the log and see that no record of the access appears.

      We fixed the problem locally by adding another call to add_to_log in the display function in mod/resource/type/file/resource.class.php. We added it immediately after this objectframe condition

      if ($resource->options == "objectframe") {

      add_to_log($course->id, "resource", "view", "view.php?id={$cm->id}", $resource->id, $cm->id);

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            jackermann Jakob Ackermann added a comment -

            The same thing happens if you use "Same Window" on Version 1.9.9
            For us I moved the line:
            add_to_log($course->id, "resource", "view", "view.php?id={$cm->id}", $resource->id, $cm->id);

            after: /// Set up some shorthand variables
            $cm = $this->cm;
            $course = $this->course;
            $resource = $this->resource;

            Show
            jackermann Jakob Ackermann added a comment - The same thing happens if you use "Same Window" on Version 1.9.9 For us I moved the line: add_to_log($course->id, "resource", "view", "view.php?id={$cm->id}", $resource->id, $cm->id); after: /// Set up some shorthand variables $cm = $this->cm; $course = $this->course; $resource = $this->resource;
            Hide
            blepoxp Glenn Ansley added a comment -

            Needed this for our install so figured I'd make a patch.

            Show
            blepoxp Glenn Ansley added a comment - Needed this for our install so figured I'd make a patch.
            Hide
            andyjdavis Andrew Davis added a comment - - edited

            Looks good to me. Thanks for the fix

            Glenn, I wasn't sure if you have the ability to put this up for integration yourself so I have done it on your behalf.

            Show
            andyjdavis Andrew Davis added a comment - - edited Looks good to me. Thanks for the fix Glenn, I wasn't sure if you have the ability to put this up for integration yourself so I have done it on your behalf.
            Hide
            stronk7 Eloy Lafuente (stronk7) 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
            stronk7 Eloy Lafuente (stronk7) 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
            blepoxp Glenn Ansley added a comment -

            No problem. Done... i think

            Show
            blepoxp Glenn Ansley added a comment - No problem. Done... i think
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks guys this has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks guys this has been integrated now
            Hide
            rwijaya Rossiani Wijaya added a comment -

            It is working great.

            Thanks for fixing it.

            Test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - It is working great. Thanks for fixing it. Test passed.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many thanks for the hard work developing and testing this. It has been spread to cvs and git upstream repositories.

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work developing and testing this. It has been spread to cvs and git upstream repositories. Closing, ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  28/Nov/11