Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29213

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

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: 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

          Attachments

            Issue Links

              Activity

              Hide
              danmarsden 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
              danmarsden 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
              kstokking 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
              kstokking 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
              danmarsden 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
              danmarsden 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
              danmarsden 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
              danmarsden 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
              kstokking 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
              kstokking 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
              danmarsden Dan Marsden added a comment -

              Hi Kris - how did you go with the patch?

              Show
              danmarsden Dan Marsden added a comment - Hi Kris - how did you go with the patch?
              Hide
              kstokking 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
              kstokking 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
              danmarsden 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
              danmarsden 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
              danmarsden 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
              danmarsden 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
              kstokking 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
              kstokking 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
              timhunt 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
              timhunt 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
              danmarsden 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
              danmarsden 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 Eloy Lafuente (stronk7) added a comment -

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

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

              Thanks Eloy!

              Show
              danmarsden Dan Marsden added a comment - Thanks Eloy!
              Hide
              rwijaya 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
              rwijaya 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
              nebgor 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
              nebgor 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
              stronk7 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
              stronk7 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
              kstokking 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
              kstokking 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
              danmarsden Dan Marsden added a comment -

              sorry - my bad - added test instructions above.

              Show
              danmarsden Dan Marsden added a comment - sorry - my bad - added test instructions above.
              Hide
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              quen 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
              quen 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
              quen 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
              quen 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 Eloy Lafuente (stronk7) added a comment -

              (passing this under my responsibility)

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

              (TODO comment added to master branch)

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - (TODO comment added to master branch)
              Hide
              stronk7 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
              stronk7 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
              danmarsden 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
              danmarsden 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
              danmarsden 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
              danmarsden 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
              stronk7 Eloy Lafuente (stronk7) added a comment -

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

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - yeah, that's the cause MDL-30039 needs to be implemented to fix this properly.
              Hide
              danvonfeldt 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
              danvonfeldt 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
              danmarsden 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
              danmarsden 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
              fsgaston 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
              fsgaston 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
              danmarsden 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
              danmarsden 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:
                    Fix Release Date:
                    28/Nov/11