Moodle
  1. Moodle
  2. MDL-30294

New rubric grading form icon not appearing in IE

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.2
    • Component/s: Grading methods
    • Labels:
    • Environment:
      IE9 under Windows using XAMPP
    • Rank:
      32639

      Description

      I noted that the icon to create a new rubric was not displaying in IE.

      Replication steps:

      1. Create a new assignment
      2. Choose Rubric as the Grading method
      3. Click Save and display

      What should be a shiny blank page icon appears as in the attached screenshot.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          This would normally be a relatively minor issue, but as it's related to the upcoming release, I've bumped it a bit.

          Show
          Michael de Raadt added a comment - This would normally be a relatively minor issue, but as it's related to the upcoming release, I've bumped it a bit.
          Hide
          Andrew Nicols added a comment -

          Confirmed under IE9.

          Viewing the image directly generates a stacktrace before the binary PNG data:

          Warning: mkdir(): File exists in /home/nicols/git/software/moodle/lib/setuplib.php on line 1098
          
          Call Stack:
              0.0005     736616   1. {main}() /home/nicols/git/software/moodle/theme/image.php:0
              0.1167   23665376   2. check_dir_exists() /home/nicols/git/software/moodle/theme/image.php:118
              0.1167   23665616   3. mkdir() /home/nicols/git/software/moodle/lib/setuplib.php:1098
          
          
          Show
          Andrew Nicols added a comment - Confirmed under IE9. Viewing the image directly generates a stacktrace before the binary PNG data: Warning: mkdir(): File exists in /home/nicols/git/software/moodle/lib/setuplib.php on line 1098 Call Stack: 0.0005 736616 1. {main}() /home/nicols/git/software/moodle/theme/image.php:0 0.1167 23665376 2. check_dir_exists() /home/nicols/git/software/moodle/theme/image.php:118 0.1167 23665616 3. mkdir() /home/nicols/git/software/moodle/lib/setuplib.php:1098
          Hide
          Andrew Nicols added a comment -

          After forcing a refresh on the image in IE, I'm no longer getting the error and it's displaying correctly. Purging the caches doesn't seem to cause it to happen again either.

          Show
          Andrew Nicols added a comment - After forcing a refresh on the image in IE, I'm no longer getting the error and it's displaying correctly. Purging the caches doesn't seem to cause it to happen again either.
          Hide
          David Mudrak added a comment -

          Closing. I was not able to reproduce on my machine. Andrew was able to reproduce but forced reload in the browser fixed the issue. I created MDL-30349 as I believe it was the original cause of this.

          Show
          David Mudrak added a comment - Closing. I was not able to reproduce on my machine. Andrew was able to reproduce but forced reload in the browser fixed the issue. I created MDL-30349 as I believe it was the original cause of this.
          Hide
          Petr Škoda added a comment -

          Hmm, this looks like a race condition during creation of cache dir structure from two separate requests. This should not happen on production sites because they are not supposed to print php errors and notices - they should log them only.

          Technically it should be possible to reproduce this, only the current user running the upgrade or resetting the caches manually should be always affected.

          Solution could be to add

          define('NO_DEBUG_DISPLAY', true);
          

          to each theme serving file. The only problem is that it might hide useful debug messages, but I guess it is time to do that because it is relatively well tested now...

          Show
          Petr Škoda added a comment - Hmm, this looks like a race condition during creation of cache dir structure from two separate requests. This should not happen on production sites because they are not supposed to print php errors and notices - they should log them only. Technically it should be possible to reproduce this, only the current user running the upgrade or resetting the caches manually should be always affected. Solution could be to add define('NO_DEBUG_DISPLAY', true ); to each theme serving file. The only problem is that it might hide useful debug messages, but I guess it is time to do that because it is relatively well tested now...
          Hide
          Petr Škoda added a comment -

          I tried to reproduce this by running cache purging in infinite loop and accessing the server using different browser, but failed to reproduce this - my computer is probably too slow.

          Anyway the proposed patch should resolve this and other similar issues because it send all debug/errors/notices to error log only in those scripts serving the theme files, developers must learn to use error logs....

          Thanks for the report!

          Show
          Petr Škoda added a comment - I tried to reproduce this by running cache purging in infinite loop and accessing the server using different browser, but failed to reproduce this - my computer is probably too slow. Anyway the proposed patch should resolve this and other similar issues because it send all debug/errors/notices to error log only in those scripts serving the theme files, developers must learn to use error logs.... Thanks for the report!
          Hide
          Sam Hemelryk added a comment -

          Thanks Petr - this has been integrated now.
          I was a little hesitant upon first reading this but in the end hopefully this will help reduce needless problems from warnings. Of course it will lead to errors being harder to debug, developers will need to learn to use logs, and of course we will have to now remember these changes when helping others to debug image/style problems (I for one know I've found easily solution by asking people to browse broken images in their browsers)

          Anyway
          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Petr - this has been integrated now. I was a little hesitant upon first reading this but in the end hopefully this will help reduce needless problems from warnings. Of course it will lead to errors being harder to debug, developers will need to learn to use logs, and of course we will have to now remember these changes when helping others to debug image/style problems (I for one know I've found easily solution by asking people to browse broken images in their browsers) Anyway Cheers Sam
          Hide
          Michael de Raadt added a comment -

          Test result: passed. Thanks guys.

          Show
          Michael de Raadt added a comment - Test result: passed. Thanks guys.
          Hide
          Petr Škoda added a comment -

          Originally I used the same debug output disabling in main /pluginfile.php but there were too many problems there, hopefully this is all tested a bit more now. Thanks.

          Show
          Petr Škoda added a comment - Originally I used the same debug output disabling in main /pluginfile.php but there were too many problems there, hopefully this is all tested a bit more now. Thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Closing as fixed, many thanks for your effort!

          Note that the changes related to master (2.2beta) have been already sent upstream. But the stable ones will be part of next weeklies (Wed/Thu) as usual.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your effort! Note that the changes related to master (2.2beta) have been already sent upstream. But the stable ones will be part of next weeklies (Wed/Thu) as usual. Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: