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:
    • Rank:
      6115

      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);

        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: