Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.2, 2.1, 2.2
    • Fix Version/s: 2.0.4, 2.1.1
    • Component/s: SCORM
    • Labels:
    • Testing Instructions:
      Hide

      Easiest way to test this is to create a SCORM and set "Display course structure on entry page" to "yes" - this way you don't have to enter the SCORM to test it.

      Check that the TOC displayed on the entry page displays the correct information.
      1. - grab the SCORM from MDL-26678 and check to make sure the TOC displays fine in IE 7 and FF
      (NOTE: The SCORM attached to MDL-26678 isn't a complete valid package - it won't display any real content as it has been stripped - the only thing to check is the TOC - make sure it displays correctly - the links in the TOC won't go anywhere.)

      2. - grab the SCORM package from MDL-26253 and make sure that the package loads correctly in the player and that the TOC is displayed correctly. (this test requires entering the SCORM as well)

      3 - grab this SCORM package and make sure the TOC displays correctly - (this is a bit of a weird package too - just check to make sure the TOC looks right!)
      http://moodle.org/mod/data/view.php?d=50&rid=1658

      Show
      Easiest way to test this is to create a SCORM and set "Display course structure on entry page" to "yes" - this way you don't have to enter the SCORM to test it. Check that the TOC displayed on the entry page displays the correct information. 1. - grab the SCORM from MDL-26678 and check to make sure the TOC displays fine in IE 7 and FF (NOTE: The SCORM attached to MDL-26678 isn't a complete valid package - it won't display any real content as it has been stripped - the only thing to check is the TOC - make sure it displays correctly - the links in the TOC won't go anywhere.) 2. - grab the SCORM package from MDL-26253 and make sure that the package loads correctly in the player and that the TOC is displayed correctly. (this test requires entering the SCORM as well) 3 - grab this SCORM package and make sure the TOC displays correctly - (this is a bit of a weird package too - just check to make sure the TOC looks right!) http://moodle.org/mod/data/view.php?d=50&rid=1658
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull Master Branch:
      master_MDL-26942
    • Rank:
      16512

      Description

      the function scorm_get_toc is duplicated in aicclib.php, scorm_12lib.php, scorm_13lib.php and has some strange handling of $scoid and $sco->id

      There should be a lot less duplication and better use of a single global function.

        Issue Links

          Activity

          Hide
          Dan Marsden added a comment -

          here's the first version of this - there were a few inconsistencies between the 3 versions of the file - only 2 "differences" were actually needed - one for cmi.exit vs cmi.core.exit and the other for checking prereq stuff.

          The function still needs some tidy up, but at least we only have one version to maintain!!!

          I expect the attached issue is still a problem so this needs a bit of testing before I submit for integration.

          Show
          Dan Marsden added a comment - here's the first version of this - there were a few inconsistencies between the 3 versions of the file - only 2 "differences" were actually needed - one for cmi.exit vs cmi.core.exit and the other for checking prereq stuff. The function still needs some tidy up, but at least we only have one version to maintain!!! I expect the attached issue is still a problem so this needs a bit of testing before I submit for integration.
          Hide
          Dan Marsden added a comment -

          NOTE FOR INTEGRATOR:
          This code still needs a bit of work(as does the rest of SCORM!) - this patch just consolidates it, fixes the inconsistencies between the 3 versions and fixes some issues with the structure of the lists (missing end tags etc) which causes yui treeview to throw js errors - Ideally I'd like to eventually split this into 2 functions - one that generates an object with the scoes in the correct order, and the 2nd which parses that object for display - turning it into html.

          Show
          Dan Marsden added a comment - NOTE FOR INTEGRATOR: This code still needs a bit of work(as does the rest of SCORM!) - this patch just consolidates it, fixes the inconsistencies between the 3 versions and fixes some issues with the structure of the lists (missing end tags etc) which causes yui treeview to throw js errors - Ideally I'd like to eventually split this into 2 functions - one that generates an object with the scoes in the correct order, and the 2nd which parses that object for display - turning it into html.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated all given branches (20, 21 and master). Hope you won't need ever to split that back.

          Thanks and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated all given branches (20, 21 and master). Hope you won't need ever to split that back. Thanks and ciao
          Hide
          Sam Hemelryk added a comment -

          Hi Dan,

          I was just testing this now.
          The end out come appeared fine, however when adding the scorm object from MDL-26678 I found I was getting hundreds of notices (didn't matter what Display course structure on entry page was set to).
          I tested with other scorm objects and they worked fine, not producing any notices.

          I've attached an HTML file which is as much of the output buffer between redirects that I managed to catch.

          I don't really know the consequences of the notices could you please check it out and let us know what is likely to have caused them/gone wrong.
          As far as I could tell things still came out fine - but having not seen that scorm object before I can't be sure.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Dan, I was just testing this now. The end out come appeared fine, however when adding the scorm object from MDL-26678 I found I was getting hundreds of notices (didn't matter what Display course structure on entry page was set to). I tested with other scorm objects and they worked fine, not producing any notices. I've attached an HTML file which is as much of the output buffer between redirects that I managed to catch. I don't really know the consequences of the notices could you please check it out and let us know what is likely to have caused them/gone wrong. As far as I could tell things still came out fine - but having not seen that scorm object before I can't be sure. Cheers Sam
          Hide
          Dan Marsden added a comment -

          Thanks Sam - I'm pretty sure that's because the package attached to MDL-26678 isn't complete - the reporter stripped out the actual content of the package and left the manifest in place - saying that we should probably open a new bug to tidy up those errors when an invalid package is provided.

          Show
          Dan Marsden added a comment - Thanks Sam - I'm pretty sure that's because the package attached to MDL-26678 isn't complete - the reporter stripped out the actual content of the package and left the manifest in place - saying that we should probably open a new bug to tidy up those errors when an invalid package is provided.
          Hide
          Sam Hemelryk added a comment -

          Ok cool thanks Dan - I'll put this back round now then and pass it. If you could please open a bug to deal with invalid packages one way or the other

          Show
          Sam Hemelryk added a comment - Ok cool thanks Dan - I'll put this back round now then and pass it. If you could please open a bug to deal with invalid packages one way or the other
          Hide
          Sam Hemelryk added a comment -

          Pass!!!! sorry about the spam everyone

          Show
          Sam Hemelryk added a comment - Pass!!!! sorry about the spam everyone
          Hide
          Sam Hemelryk added a comment -

          Congratulations - this fix has just been released in the weeklies.

          Show
          Sam Hemelryk added a comment - Congratulations - this fix has just been released in the weeklies.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: