Uploaded image for project: '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

          Attachments

            Activity

            Hide
            lazydaisy 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
            lazydaisy 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
            danmarsden 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
            danmarsden 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
            lazydaisy 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
            lazydaisy 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
            samhemelryk 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
            samhemelryk 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
            fred Frédéric Massart added a comment -

            Test passed on 2.3 onwards. Thanks!

            Show
            fred Frédéric Massart added a comment - Test passed on 2.3 onwards. Thanks!
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  11/Mar/13