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
    • Rank:
      18756

      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.

      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: