Moodle
  1. Moodle
  2. MDL-37696

SCORM module includes a CSS file twice

    Details

      Description

      When you visit the page:

      mod/scorm/player.php

      The file mod/scorm/styles.css is loaded twice, once by whatever mechanism gathers together the style.css file from various modules and turns them into one big CSS file and a second time by the player.php script which requires the file on line 193.

      As well as making it harder (and more confusing) to theme the SCORM module, this is an unnecessary performance hit (though probably negligible in terms of magnitude).

        Gliffy Diagrams

          Activity

          Hide
          Mary Evans added a comment -

          @Dan Marsden

          I think what David is trying to say is that should a theme designer want to exclude a plugin stylesheet using this code in a theme's config.php...

          $THEME->plugins_exclude_sheets = array('mod' => array('scorm'), 'block' => array('rss'));

          becomes very difficult when you come to exclude mod/scorm/style.css because of the hard coding in player.php.

          Show
          Mary Evans added a comment - @ Dan Marsden I think what David is trying to say is that should a theme designer want to exclude a plugin stylesheet using this code in a theme's config.php... $THEME->plugins_exclude_sheets = array('mod' => array('scorm'), 'block' => array('rss')); becomes very difficult when you come to exclude mod/scorm/style.css because of the hard coding in player.php.
          Hide
          Dan Marsden added a comment -

          I'm not sure if this should be backported to stable branches - here's the patch for master - happy for integrators to backport if they want to otherwise I'm just as happy for this to be master only.

          Show
          Dan Marsden added a comment - I'm not sure if this should be backported to stable branches - here's the patch for master - happy for integrators to backport if they want to otherwise I'm just as happy for this to be master only.
          Hide
          Mary Evans added a comment -

          That will fix the problem David reported. Hoverever, I don't have a scorm package to test it with or I would do so gladly.

          Show
          Mary Evans added a comment - That will fix the problem David reported. Hoverever, I don't have a scorm package to test it with or I would do so gladly.
          Hide
          Sam Hemelryk added a comment -

          Thanks Dan this has been integrated now.

          I've chosen to backport this given its a simple + safe patch that will benefit performance (even if just marginally).

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Dan this has been integrated now. I've chosen to backport this given its a simple + safe patch that will benefit performance (even if just marginally). Many thanks Sam
          Hide
          Frédéric Massart added a comment -

          Test passed on 2.3 onwards. Thanks!

          Show
          Frédéric Massart added a comment - Test passed on 2.3 onwards. Thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed.

          (and won't be revisiting it unless some regression is found)

          Thanks and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed. (and won't be revisiting it unless some regression is found) Thanks and ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: