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

Images failing in site events in calendar

    Details

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

      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)

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            salvetore 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
            salvetore 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 s.kavita added a comment -

            Hi Michael

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

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

            Thanks for adding more to that.

            Show
            salvetore Michael de Raadt added a comment - Thanks for adding more to that.
            Hide
            s.kavita 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 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
            inderjeetsinghit 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
            inderjeetsinghit 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
            gsisson 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
            gsisson 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
            salvetore Michael de Raadt added a comment -

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

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

            Thanks Michael.

            Show
            gsisson Glenn Sisson added a comment - Thanks Michael.
            Hide
            fred 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
            fred 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
            markn 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
            markn 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
            fred Frédéric Massart added a comment -

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

            Show
            fred Frédéric Massart added a comment - Thanks Mark. I changed the comment and am now pushing for integration. Cheers!
            Hide
            poltawski 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
            poltawski 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
            fred 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
            fred 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
            poltawski Dan Poltawski added a comment -

            Integrated, thanks Fred.

            Show
            poltawski Dan Poltawski added a comment - Integrated, thanks Fred.
            Hide
            skodak Petr Skoda added a comment -

            works in all branches, thanks

            Show
            skodak Petr Skoda added a comment - works in all branches, thanks
            Hide
            poltawski 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
            poltawski 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:
                  Fix Release Date:
                  14/Jan/13