Moodle
  1. Moodle
  2. MDL-23047

Accessing file resources without frame fails to create log entry

    Details

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

          Activity

          Hide
          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
          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
          Glenn Ansley added a comment -

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

          Show
          Glenn Ansley added a comment - Needed this for our install so figured I'd make a patch.
          Hide
          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
          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
          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
          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
          Glenn Ansley added a comment -

          No problem. Done... i think

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

          Thanks guys this has been integrated now

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

          It is working great.

          Thanks for fixing it.

          Test passed.

          Show
          Rossiani Wijaya added a comment - It is working great. Thanks for fixing it. Test passed.
          Hide
          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
          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: