Moodle
  1. Moodle
  2. MDL-29213

Implement custom option in SCORM module settings to force IE8 rendering when needed

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.13, 2.0.5, 2.1.2, 2.2
    • Fix Version/s: 2.0.6, 2.1.3
    • Component/s: SCORM
    • Labels:
    • Testing Instructions:
      Hide

      Please install this Scorm package
      http://moodle.org/mod/data/view.php?d=50&rid=1655&filter=1

      enter scorm using a browser other than IE 9 - check the source code of the player.php page and make sure the following text doesn't appear in the <head>
      <meta http-equiv="X-UA-Compatible" content="IE=8" />

      Enter the scorm using IE9 and check to make sure the above code appears in the source code of player.php page.

      in IE 9 complete the SCORM package - then check to make sure an attempt is registered on the reports page.

      NOTE: ignore any weird looking text in the TOC - there's a bug with latest version of YUI Treeview that screws up the list in the toc a bit.

      Show
      Please install this Scorm package http://moodle.org/mod/data/view.php?d=50&rid=1655&filter=1 enter scorm using a browser other than IE 9 - check the source code of the player.php page and make sure the following text doesn't appear in the <head> <meta http-equiv="X-UA-Compatible" content="IE=8" /> Enter the scorm using IE9 and check to make sure the above code appears in the source code of player.php page. in IE 9 complete the SCORM package - then check to make sure an attempt is registered on the reports page. NOTE: ignore any weird looking text in the TOC - there's a bug with latest version of YUI Treeview that screws up the list in the toc a bit.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull Master Branch:
      master_MDL-29213

      Description

      Due to various issues with IE8&9 and their interpretation of scorm javascript lot of times we are forced to add famous "force IE7 rendering" tag. This change add's that as a nice module setting that is disabled by default.

        Gliffy Diagrams

        1. force_ie7.patch
          3 kB
          Kris Stokking
        2. moodle19stable_scorm_ie7patch.patch
          4 kB
          Darko Miletic

          Issue Links

            Activity

            Hide
            Dan Marsden added a comment -

            Hi Darko,

            I need some more specific information on the issues you are having with IE 8/9 - are you sure you don't have any other custom code installed that might be causing problems?

            I know of several people using IE8 and IE9 that have used SCORM succesfully - if there is a problem somewhere else I'd prefer to fix the underlying issue rather than adding a hack to force IE 7 mode.

            Show
            Dan Marsden added a comment - Hi Darko, I need some more specific information on the issues you are having with IE 8/9 - are you sure you don't have any other custom code installed that might be causing problems? I know of several people using IE8 and IE9 that have used SCORM succesfully - if there is a problem somewhere else I'd prefer to fix the underlying issue rather than adding a hack to force IE 7 mode.
            Hide
            Kris Stokking added a comment -

            Hi Dan,

            When certain SCO's are launched in IE9, the SCO is not communicating any information to the LMS, and the state is always reported as incomplete without any score. We've experienced a problem with Articulate generated SCORMs specificially, but I believe the issue is more widespread than that. See http://www.articulate.com/support/presenter09/kb/?p=3693 and http://bugs.adobe.com/jira/browse/FP-6569 for more information.

            The supplied patch does not work correctly - we are working on an updated version. We have confirmed that forcing Compatibility Mode sitewide does resolve the issue with no work on the client side. We're working on a patch that forces Compatibility Mode for the SCORM module only.

            Thanks,
            Kris

            Show
            Kris Stokking added a comment - Hi Dan, When certain SCO's are launched in IE9, the SCO is not communicating any information to the LMS, and the state is always reported as incomplete without any score. We've experienced a problem with Articulate generated SCORMs specificially, but I believe the issue is more widespread than that. See http://www.articulate.com/support/presenter09/kb/?p=3693 and http://bugs.adobe.com/jira/browse/FP-6569 for more information. The supplied patch does not work correctly - we are working on an updated version. We have confirmed that forcing Compatibility Mode sitewide does resolve the issue with no work on the client side. We're working on a patch that forces Compatibility Mode for the SCORM module only. Thanks, Kris
            Hide
            Dan Marsden added a comment -

            Hi Kris,

            what Moodle version are you using? - are you using the "new window" option for your SCORMS? - can you test a version of 2.0.5+ or 2.1.2+ to see if you can reproduce it in those versions?

            MDL-28295 has been applied to the above mentioned versions but this hasn't been backported to the 1.9Stable branch

            forcing compatibility mode doesn't sound like a real solution to me - we need to find more specific information where this is failing so we can fix the root issue rather than using a hack to set compatibility mode.

            Show
            Dan Marsden added a comment - Hi Kris, what Moodle version are you using? - are you using the "new window" option for your SCORMS? - can you test a version of 2.0.5+ or 2.1.2+ to see if you can reproduce it in those versions? MDL-28295 has been applied to the above mentioned versions but this hasn't been backported to the 1.9Stable branch forcing compatibility mode doesn't sound like a real solution to me - we need to find more specific information where this is failing so we can fix the root issue rather than using a hack to set compatibility mode.
            Hide
            Dan Marsden added a comment -

            going and reading the flash bug which has been shifted to their new tracker here:
            https://bugbase.adobe.com/index.cfm?event=bug&id=2928139

            looks like forcing the compatibility mode is all we can do...

            Show
            Dan Marsden added a comment - going and reading the flash bug which has been shifted to their new tracker here: https://bugbase.adobe.com/index.cfm?event=bug&id=2928139 looks like forcing the compatibility mode is all we can do...
            Hide
            Kris Stokking added a comment -

            looks like forcing the compatibility mode is all we can do...

            That's right - we're providing a workaround for a bug in Flash. We hope to have the patch finalized by tonight.

            Show
            Kris Stokking added a comment - looks like forcing the compatibility mode is all we can do... That's right - we're providing a workaround for a bug in Flash. We hope to have the patch finalized by tonight.
            Hide
            Dan Marsden added a comment -

            Hi Kris - how did you go with the patch?

            Show
            Dan Marsden added a comment - Hi Kris - how did you go with the patch?
            Hide
            Kris Stokking added a comment -

            Dan, thanks for reminding me! The following patch is for Moodle 1.9 - we created an Experimental Setting that only kicks in on SCORM pages. The patch had to be made to weblib.php because of the way the META tags need to be defined in the header. Let me know if you have any questions.

            Show
            Kris Stokking added a comment - Dan, thanks for reminding me! The following patch is for Moodle 1.9 - we created an Experimental Setting that only kicks in on SCORM pages. The patch had to be made to weblib.php because of the way the META tags need to be defined in the header. Let me know if you have any questions.
            Hide
            Dan Marsden added a comment -

            HI Kris - I see you've done this to all browsers and have set it to IE 7 - have you been able to duplicate this issue in other browsers? - or just IE 9?

            thanks,

            Show
            Dan Marsden added a comment - HI Kris - I see you've done this to all browsers and have set it to IE 7 - have you been able to duplicate this issue in other browsers? - or just IE 9? thanks,
            Hide
            Dan Marsden added a comment -

            Note to integrator - this fix is intentionally for 2.X branches only. We can backport this later on a seperate Tracker issue if needed.

            This hijacks $CFG a bit but there is no other easy way to conditionally inject generic meta into the head - only js/refresh stuff - and this fix probably doesn't justify a new injection point into standard_head_html()

            Show
            Dan Marsden added a comment - Note to integrator - this fix is intentionally for 2.X branches only. We can backport this later on a seperate Tracker issue if needed. This hijacks $CFG a bit but there is no other easy way to conditionally inject generic meta into the head - only js/refresh stuff - and this fix probably doesn't justify a new injection point into standard_head_html()
            Hide
            Kris Stokking added a comment -

            HI Kris - I see you've done this to all browsers and have set it to IE 7 - have you been able to duplicate this issue in other browsers? - or just IE 9?

            Only IE9. Other browsers (FF, Chrome) are unaffected by the patch.

            Show
            Kris Stokking added a comment - HI Kris - I see you've done this to all browsers and have set it to IE 7 - have you been able to duplicate this issue in other browsers? - or just IE 9? Only IE9. Other browsers (FF, Chrome) are unaffected by the patch.
            Hide
            Tim Hunt added a comment -

            That is a truly horrible hack, but it is entirely local to the SCORM module, so I won't object.

            Show
            Tim Hunt added a comment - That is a truly horrible hack, but it is entirely local to the SCORM module, so I won't object.
            Hide
            Dan Marsden added a comment -

            Yep - quite happy for an integrator to reject but only if an alternative way of injecting meta in the header is provided

            Show
            Dan Marsden added a comment - Yep - quite happy for an integrator to reject but only if an alternative way of injecting meta in the header is provided
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Uhm, perhaps it would be worth having some way (method) to inject any link|meta tag there in the renderer instead of hacking $CFG->additionalhtmlhead (much like the css/jss stuff but surely would be simpler).

            Also, it should be possible to create one mod_scorm renderer, instead of using the core one:

            $scormoutput = $PAGE->get_renderer('mod_scorm');
            $scormoutput->header()|footer()|box()...

            and then, simply, in the mod_scorm renderer, override the standard_head_html() method to inject your own stuff.

            public function standard_head_html() {
                return '<meta eloy="crazy ideas" />' . parent::standard_head_html();
            }
            

            But sadly, that does not work because themes go straight to $OUTPUT->standard_head_html() (core renderer and not our own). Epic fail IMO, (surely conflict between mod/theme renderers prevent doing that or so) but that's another story...

            So I'm going to integrate this with current (agreed) nasty hack, thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - Uhm, perhaps it would be worth having some way (method) to inject any link|meta tag there in the renderer instead of hacking $CFG->additionalhtmlhead (much like the css/jss stuff but surely would be simpler). Also, it should be possible to create one mod_scorm renderer, instead of using the core one: $scormoutput = $PAGE->get_renderer('mod_scorm'); $scormoutput->header()|footer()|box()... and then, simply, in the mod_scorm renderer, override the standard_head_html() method to inject your own stuff. public function standard_head_html() { return '<meta eloy="crazy ideas" />' . parent::standard_head_html(); } But sadly, that does not work because themes go straight to $OUTPUT->standard_head_html() (core renderer and not our own). Epic fail IMO, (surely conflict between mod/theme renderers prevent doing that or so) but that's another story... So I'm going to integrate this with current (agreed) nasty hack, thanks!
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks!

            Note that for master, I've amended the commit to add one missing whitespace line, so prob... you won't find it when cleaning your dev branches.

            Show
            Eloy Lafuente (stronk7) added a comment - Integrated, thanks! Note that for master, I've amended the commit to add one missing whitespace line, so prob... you won't find it when cleaning your dev branches.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Also, created MDL-30039 about being able to inject some tags programmatically, FYI, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Also, created MDL-30039 about being able to inject some tags programmatically, FYI, ciao
            Hide
            Dan Marsden added a comment -

            Thanks Eloy!

            Show
            Dan Marsden added a comment - Thanks Eloy!
            Hide
            Rossiani Wijaya added a comment -

            When tested this with IE9, student's grade didn't get recorded correctly and the attempted status is set as 'not attempted'

            Could someone else test and confirm this?

            Show
            Rossiani Wijaya added a comment - When tested this with IE9, student's grade didn't get recorded correctly and the attempted status is set as 'not attempted' Could someone else test and confirm this?
            Hide
            Aparup Banerjee added a comment -

            i think we need some test instructions here to help differentiate passing this patch from other possible issues seen.

            Show
            Aparup Banerjee added a comment - i think we need some test instructions here to help differentiate passing this patch from other possible issues seen.
            Hide
            Eloy Lafuente (stronk7) added a comment - - edited

            grrr, my fault. I forgot to review that point on integration!

            Surely thin comment is the best one explaining the problem and the testing needed:

            http://tracker.moodle.org/browse/MDL-29213?focusedCommentId=125022&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-125022

            And the original bug is here: https://bugbase.adobe.com/index.cfm?event=bug&id=2928139

            But yes, this requires proper testing instructions, example package to tests... expected results. We must assume testers are blind (they don't need to read the whole issue nor have any knowledge) and testing behavior must be clear for them. Apologises again for missing to check that.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - - edited grrr, my fault. I forgot to review that point on integration! Surely thin comment is the best one explaining the problem and the testing needed: http://tracker.moodle.org/browse/MDL-29213?focusedCommentId=125022&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-125022 And the original bug is here: https://bugbase.adobe.com/index.cfm?event=bug&id=2928139 But yes, this requires proper testing instructions, example package to tests... expected results. We must assume testers are blind (they don't need to read the whole issue nor have any knowledge) and testing behavior must be clear for them. Apologises again for missing to check that. Ciao
            Hide
            Kris Stokking added a comment -

            It's most helpful to use SCORM debugging for testing this patch:

            Without the patch: Suspend data always returns as null, and the status and grade fields are never updated.

            With the patch applied: Suspend data returns with the correct values. Grade and status fields are updated properly.

            Show
            Kris Stokking added a comment - It's most helpful to use SCORM debugging for testing this patch: Without the patch: Suspend data always returns as null, and the status and grade fields are never updated. With the patch applied: Suspend data returns with the correct values. Grade and status fields are updated properly.
            Hide
            Dan Marsden added a comment -

            sorry - my bad - added test instructions above.

            Show
            Dan Marsden added a comment - sorry - my bad - added test instructions above.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            lol, yeah, I was using the debugger to get LMSGetValue("cmi.suspend_data") and detected the TOC problem, really annoying...

            Thanks for the instructions!

            Show
            Eloy Lafuente (stronk7) added a comment - lol, yeah, I was using the debugger to get LMSGetValue("cmi.suspend_data") and detected the TOC problem, really annoying... Thanks for the instructions!
            Hide
            Eloy Lafuente (stronk7) added a comment -

            I've tried faking the Browser agent and I can confirm that the new meta:

            <meta http-equiv="X-UA-Compatible" content="IE=8" />
            

            is only set when the browser is IE9. Tried with FF, Safari and other IE versions, the meta is not set.

            Going to try with one real IE9 browser now... (hope to be able to do so).

            Show
            Eloy Lafuente (stronk7) added a comment - I've tried faking the Browser agent and I can confirm that the new meta: <meta http-equiv="X-UA-Compatible" content="IE=8" /> is only set when the browser is IE9. Tried with FF, Safari and other IE versions, the meta is not set. Going to try with one real IE9 browser now... (hope to be able to do so).
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Crap, IE9 requires VistaSP2 or newer, and I'm sticky to XP here yet.

            Looking for potential tester...

            Show
            Eloy Lafuente (stronk7) added a comment - Crap, IE9 requires VistaSP2 or newer, and I'm sticky to XP here yet. Looking for potential tester...
            Hide
            Sam Marshall added a comment -

            I completed test in IE9 (note that I scored 0 and did not see any fruit images, possibly because my maximised browser on 1024x768 is not big enough to defeat the moodle 'middle column gets chopped off, who needs scrollbars' bug) and my name appears on report. So it looks OK.

            Also I do see at the end of the head:

            <meta http-equiv="X-UA-Compatible" content="IE=8" /></head>
            

            HOWEVER.

            X-UA-Compatible is always supposed to be placed as the very first thing in head, not the last, isn't it?

            And pressing F12 reveals that my browser is in IE9 mode, not IE8 mode.

            Show
            Sam Marshall added a comment - I completed test in IE9 (note that I scored 0 and did not see any fruit images, possibly because my maximised browser on 1024x768 is not big enough to defeat the moodle 'middle column gets chopped off, who needs scrollbars' bug) and my name appears on report. So it looks OK. Also I do see at the end of the head: <meta http-equiv="X-UA-Compatible" content="IE=8" /></head> HOWEVER. X-UA-Compatible is always supposed to be placed as the very first thing in head, not the last, isn't it? And pressing F12 reveals that my browser is in IE9 mode, not IE8 mode.
            Hide
            Sam Marshall added a comment -

            forgot to say - I tested on qa.moodle.net here:

            http://qa.moodle.net/mod/scorm/view.php?id=67

            Show
            Sam Marshall added a comment - forgot to say - I tested on qa.moodle.net here: http://qa.moodle.net/mod/scorm/view.php?id=67
            Hide
            Eloy Lafuente (stronk7) added a comment - - edited

            Yeah, just found this:

            "The X-UA-Compatible header is not case sensitive; however, it must appear in the header of the webpage (the HEAD section) before all other elements except for the title element and other meta elements."
            (source: http://msdn.microsoft.com/en-us/library/cc288325)

            So perhaps we should revert this and try next week? Dan?

            Many thanks Sam! Ciao

            Edited: I think the requirement to be on top of the <head> in not achievable using $CFG->additionalhtmlhead (it's always added at the end), so we have arrived to a dead end, hehe.

            Edited2: So should we implement MDL-30039 ASAP instead, and then use it to fix this?

            Edited3: Or should we pass this (doesn't break anything) with TODO comment in code about MDL-30039 needed to fix it properly and done?

            +3 needed to decide between Edited2 and Edited3 alternatives.

            Show
            Eloy Lafuente (stronk7) added a comment - - edited Yeah, just found this: "The X-UA-Compatible header is not case sensitive; however, it must appear in the header of the webpage (the HEAD section) before all other elements except for the title element and other meta elements." (source: http://msdn.microsoft.com/en-us/library/cc288325 ) So perhaps we should revert this and try next week? Dan? Many thanks Sam! Ciao Edited: I think the requirement to be on top of the <head> in not achievable using $CFG->additionalhtmlhead (it's always added at the end), so we have arrived to a dead end, hehe. Edited2: So should we implement MDL-30039 ASAP instead, and then use it to fix this? Edited3: Or should we pass this (doesn't break anything) with TODO comment in code about MDL-30039 needed to fix it properly and done? +3 needed to decide between Edited2 and Edited3 alternatives.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Oki, +3 for Edited3 alternative. At worst, current patch does nothing (bad nor good), so...

            • I'm adding TODO comment pointing to MDL-30039 in the master branch.
            • I'm closing this as integrated, if the issue is not fixed or needs other changes, please open new issue.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Oki, +3 for Edited3 alternative. At worst, current patch does nothing (bad nor good), so... I'm adding TODO comment pointing to MDL-30039 in the master branch. I'm closing this as integrated, if the issue is not fixed or needs other changes, please open new issue. Ciao
            Hide
            Eloy Lafuente (stronk7) added a comment -

            (passing this under my responsibility)

            Show
            Eloy Lafuente (stronk7) added a comment - (passing this under my responsibility)
            Hide
            Eloy Lafuente (stronk7) added a comment -

            (TODO comment added to master branch)

            Show
            Eloy Lafuente (stronk7) added a comment - (TODO comment added to master branch)
            Hide
            Eloy Lafuente (stronk7) added a comment -

            And this has been sent upstream (already available @ git and cvs repos). Many, many thanks!

            Closing as fixed, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - And this has been sent upstream (already available @ git and cvs repos). Many, many thanks! Closing as fixed, ciao
            Hide
            Dan Marsden added a comment -

            Thanks Eloy - given that it's not actually doing anything I might have been tempted to revert. Sam - thanks for the info on placement of the tag - didn't realise there was a condition there

            Show
            Dan Marsden added a comment - Thanks Eloy - given that it's not actually doing anything I might have been tempted to revert. Sam - thanks for the info on placement of the tag - didn't realise there was a condition there
            Hide
            Dan Marsden added a comment -

            arg - there aren't any hooks to allow us to print something at the top of <head> either - each theme prints stuff in <HEAD> before calling a core function....

            Show
            Dan Marsden added a comment - arg - there aren't any hooks to allow us to print something at the top of <head> either - each theme prints stuff in <HEAD> before calling a core function....
            Hide
            Eloy Lafuente (stronk7) added a comment -

            yeah, that's the cause MDL-30039 needs to be implemented to fix this properly.

            Show
            Eloy Lafuente (stronk7) added a comment - yeah, that's the cause MDL-30039 needs to be implemented to fix this properly.
            Hide
            Dan VonFeldt added a comment -

            I have added the additionalhtmlhead code diff mentioned and this is still not forcing compatibility mode. Please advise on what I can do to resolve this issue. Thanks.

            Show
            Dan VonFeldt added a comment - I have added the additionalhtmlhead code diff mentioned and this is still not forcing compatibility mode. Please advise on what I can do to resolve this issue. Thanks.
            Hide
            Dan Marsden added a comment -

            Hi Dan - you will need to modify your theme's header.html file and add the stuff manually yourself - make sure it appears as the "first" item after the <head> tag in your header file.

            Show
            Dan Marsden added a comment - Hi Dan - you will need to modify your theme's header.html file and add the stuff manually yourself - make sure it appears as the "first" item after the <head> tag in your header file.
            Hide
            Forrest Gaston added a comment -

            Dan Marsden,

            I just need some clarification, is this problem fixed in Moodle 2.2 or is it still an issue, I'm running into this problem in testing with 2.2 and just needed some clarification.

            Thanks
            Forrest

            Show
            Forrest Gaston added a comment - Dan Marsden, I just need some clarification, is this problem fixed in Moodle 2.2 or is it still an issue, I'm running into this problem in testing with 2.2 and just needed some clarification. Thanks Forrest
            Hide
            Dan Marsden added a comment -

            Hi Forrest - unfortunately this doesn't really help much - to implement this you need to modify your themes header file and add the meta tag there yourself.
            keep an eye on MDL-30039 for an eventual solution that might allow us to set it easier.

            Show
            Dan Marsden added a comment - Hi Forrest - unfortunately this doesn't really help much - to implement this you need to modify your themes header file and add the meta tag there yourself. keep an eye on MDL-30039 for an eventual solution that might allow us to set it easier.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: