Moodle
  1. Moodle
  2. MDL-30731

Images failing in site events in calendar

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.3.1
    • Fix Version/s: 2.2.7, 2.3.4
    • Component/s: Calendar
    • Labels:
    • Database:
      MySQL
    • Testing Instructions:
      Hide

      Test pre-requisites

      • 1 event of each type with an image in their description

      Test steps

      1. Login as an admin and navigate view each event.
      2. Make sure sure you can see the image in their description
      3. Logout and view the calendar, make sure you can see the content of the site event
      4. Login back as an admin
      5. Add an image to the user and site event
      6. Repeat step 1 to 3
      Show
      Test pre-requisites 1 event of each type with an image in their description Test steps Login as an admin and navigate view each event. Make sure sure you can see the image in their description Logout and view the calendar, make sure you can see the content of the site event Login back as an admin Add an image to the user and site event Repeat step 1 to 3
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30731-master
    • Rank:
      33608

      Description

      I have created a site event and try to upload an image with that event. The image was successfully uploaded but when i see that event later on then the image is not displaying in any browser(Firefox, Chrome,Opera, I.E 8)

      1. calendar1.png
        167 kB
      2. calendar2.png
        43 kB

        Activity

        Hide
        Michael de Raadt added a comment -

        Hi.

        Could you please provide some more detail? Perhaps you could create a set of replication steps and take screenshots.

        Show
        Michael de Raadt added a comment - Hi. Could you please provide some more detail? Perhaps you could create a set of replication steps and take screenshots.
        Hide
        s.kavita added a comment -

        Hi Michael

        I have attached the screen - shot. Please check it.

        Show
        s.kavita added a comment - Hi Michael I have attached the screen - shot. Please check it.
        Hide
        Michael de Raadt added a comment -

        Thanks for adding more to that.

        Show
        Michael de Raadt added a comment - Thanks for adding more to that.
        Hide
        s.kavita added a comment -

        Hello Michael

        Video is also not working. Please have a look and try to solve this issue soon.

        Thanks,
        Kavita

        Show
        s.kavita added a comment - Hello Michael Video is also not working. Please have a look and try to solve this issue soon. Thanks, Kavita
        Hide
        inderjeet added a comment -

        s.kavita I think you need double check because I am able to do what you were expecting calender event to do using image. I might not be sure in case of Remote Server. If that's the case I can check the thing twice also.

        Show
        inderjeet added a comment - s.kavita I think you need double check because I am able to do what you were expecting calender event to do using image. I might not be sure in case of Remote Server. If that's the case I can check the thing twice also.
        Hide
        Glenn Sisson added a comment - - edited

        I first note this issue when I upgraded to 2.2.3 and then subsequently upgraded to 2.3.1.

        This is also effecting files uploaded to a site event such as a pdf. If I upload the pdf to another location (forum, course etc) and link from the calendar site event it works ok.

        I am also having the same image problem shown as broken links. If I click on a file link I get the following debug message:

        Debug info:
        Error code: filenotfound
        Stack trace:
        line 467 of \lib\setuplib.php: moodle_exception thrown
        line 1899 of \lib\filelib.php: call to print_error()
        line 3576 of \lib\filelib.php: call to send_file_not_found()
        line 38 of \pluginfile.php: call to file_pluginfile()

        This issue does not occur when creating user events.

        Please review priority as this issue makes the calendar site events ineffective.

        Thanks, Glenn

        Show
        Glenn Sisson added a comment - - edited I first note this issue when I upgraded to 2.2.3 and then subsequently upgraded to 2.3.1. This is also effecting files uploaded to a site event such as a pdf. If I upload the pdf to another location (forum, course etc) and link from the calendar site event it works ok. I am also having the same image problem shown as broken links. If I click on a file link I get the following debug message: Debug info: Error code: filenotfound Stack trace: line 467 of \lib\setuplib.php: moodle_exception thrown line 1899 of \lib\filelib.php: call to print_error() line 3576 of \lib\filelib.php: call to send_file_not_found() line 38 of \pluginfile.php: call to file_pluginfile() This issue does not occur when creating user events. Please review priority as this issue makes the calendar site events ineffective. Thanks, Glenn
        Hide
        Michael de Raadt added a comment -

        I'm bumping this issue. This is still a problem in current supported versions.

        Show
        Michael de Raadt added a comment - I'm bumping this issue. This is still a problem in current supported versions.
        Hide
        Glenn Sisson added a comment -

        Thanks Michael.

        Show
        Glenn Sisson added a comment - Thanks Michael.
        Hide
        Frédéric Massart added a comment -

        Pushing this for peer review.

        There were two issues:

        • The context was not properly set, therefore the image was stored in the wrong place in the file area.
        • The pluginfile function could not serve the content of site events.
        Show
        Frédéric Massart added a comment - Pushing this for peer review. There were two issues: The context was not properly set, therefore the image was stored in the wrong place in the file area. The pluginfile function could not serve the content of site events.
        Hide
        Mark Nelson added a comment - - edited

        [Y] Syntax
        [-] Output
        [Y] Whitespace
        [-] Language
        [-] Databases
        [Y] Testing
        [-] Security
        [-] Documentation
        [Y] Git
        [Y] Sanity check

        Hi Fred, you have a check on $usingeditor within an if statement that already checks this, so I thought this was not necessary when viewing the diff, however I can see after viewing the whole file that there is a case where the switch does not meet any of the conditions and sets it back to false, so it makes sense now. Also, in lib/filelib.php you make reference in a comment to a previous if statement that checks if it's the site id, but in fact it checks if it is not the siteid, which is the check you have added. This comment gave me the impression that the previous if statement handled the scenario when it was the siteid, which is not the case. This is just my opinion, so isn't necessary to change.

        Show
        Mark Nelson added a comment - - edited [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check Hi Fred, you have a check on $usingeditor within an if statement that already checks this, so I thought this was not necessary when viewing the diff, however I can see after viewing the whole file that there is a case where the switch does not meet any of the conditions and sets it back to false, so it makes sense now. Also, in lib/filelib.php you make reference in a comment to a previous if statement that checks if it's the site id, but in fact it checks if it is not the siteid, which is the check you have added. This comment gave me the impression that the previous if statement handled the scenario when it was the siteid, which is not the case. This is just my opinion, so isn't necessary to change.
        Hide
        Frédéric Massart added a comment -

        Thanks Mark. I changed the comment and am now pushing for integration. Cheers!

        Show
        Frédéric Massart added a comment - Thanks Mark. I changed the comment and am now pushing for integration. Cheers!
        Hide
        Dan Poltawski added a comment -
        1. I'm not a big fan of this statement because it relies on peoples understanding of operator precedence:
          if ($course->id != SITEID && !is_enrolled($context) and !is_viewing($context)) {
          

          I think it would be beneficial for future developers to add some additional parenthesis to be absolutely clear.

        Also, could you humour me and explain a bit more about what was breaking the context setting from this issue. Cheers

        Show
        Dan Poltawski added a comment - I'm not a big fan of this statement because it relies on peoples understanding of operator precedence: if ($course->id != SITEID && !is_enrolled($context) and !is_viewing($context)) { I think it would be beneficial for future developers to add some additional parenthesis to be absolutely clear. Also, could you humour me and explain a bit more about what was breaking the context setting from this issue. Cheers
        Hide
        Frédéric Massart added a comment -

        Dan, you are right, this if is quite ugly. Especially the && and 'and' ...

        The problem was that the context passed to the text editor was not right. The context of the event is calculated using the variables course AND courseid. If one fails, we rely on the other. Have a look at calculate_context and you'll get scared. So what I did is setting the right values and after that re-calculate the context to then pass it to the editor.

        Once the editor has the right context, it still needs to be able to serve the files, but the calendar file serving had no rules for the context_system level, hence the modifications in pluginlib.

        Show
        Frédéric Massart added a comment - Dan, you are right, this if is quite ugly. Especially the && and 'and' ... The problem was that the context passed to the text editor was not right. The context of the event is calculated using the variables course AND courseid. If one fails, we rely on the other. Have a look at calculate_context and you'll get scared. So what I did is setting the right values and after that re-calculate the context to then pass it to the editor. Once the editor has the right context, it still needs to be able to serve the files, but the calendar file serving had no rules for the context_system level, hence the modifications in pluginlib.
        Hide
        Dan Poltawski added a comment -

        Integrated, thanks Fred.

        Show
        Dan Poltawski added a comment - Integrated, thanks Fred.
        Hide
        Petr Škoda added a comment -

        works in all branches, thanks

        Show
        Petr Škoda added a comment - works in all branches, thanks
        Hide
        Dan Poltawski added a comment -

        Congratulations! Another bug solved.. only another 7330 to go, thanks for contributing to contributing to 0.8% of all bugs being fixed this week!

        ciao
        Dan

        Show
        Dan Poltawski added a comment - Congratulations! Another bug solved.. only another 7330 to go, thanks for contributing to contributing to 0.8% of all bugs being fixed this week! ciao Dan

          People

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

            Dates

            • Created:
              Updated:
              Resolved: