Moodle
  1. Moodle
  2. MDL-37696

SCORM module includes a CSS file twice

    Details

    • Rank:
      47406

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

        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: