Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.7
    • Fix Version/s: 1.9.9
    • Component/s: SCORM
    • Labels:
      None
    • Environment:
      Apache Linux Debian
    • Database:
      MySQL
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      26667

      Description

      Try the attached package in the latest stable Moodle environment.

      On save/suspend of the data model I receive a warning about insufficient choices for the correct_responses pattern. On debugging I found this to be a single alert() error in the code that doesn't really apply when setting the correct answer to a multi-choice question.

      However, the more concerning problem is that when the session is resumed the JavaScript outputted by Moodle is syntactically invalid and causes the whole SCORM 2004 support to fail. The affected JS is part of data model reinitialisation:

      cmi.interactions.N0 = new Object();
      cmi.interactions.N0.objectives = new Object();
      cmi.interactions.N0.objectives._children = objectives_children;
      cmi.interactions.N0.correct_responses = new Object();
      cmi.interactions.N0.correct_responses._children = correct_responses_children;
      cmi.interactions.N0.type = 'true-false';
      cmi.interactions.N0.weighting = '1';
      cmi.interactions.N0.correct_responses = new Object();
      cmi.interactions.N0.correct_responses.N0 = new Object();
      cmi.interactions.N0.correct_responses.N0.pattern = 'true';
      cmi.interactions.N0.result = 'correct';
      cmi.interactions.N0.timestamp = '2010-01-19T12:27:41';
      cmi.interactions.N0.description = 'Does lamp LP A light up?';
      cmi.interactions_N0.correct_responses._count = 1;
      cmi.interactions.N0.objectives = new Object();
      cmi.interactions.N0.objectives.N0 = new Object();
      cmi.interactions.N0.objectives.N0.id = 'urn:LJCreate:Objective-M2470001-C001-1';
      cmi.interactions.N0.id = 'urn:LJCreate:Interaction-M2470001-C001-2_1';
      cmi.interactions.N0.learner_response = 'true';
      cmi.interactions_N0.objectives._count = 1;

      The problem there is that underscores are appearing instead of object namespace dots for the ._count fields.

        Issue Links

          Activity

          Hide
          Dan Marsden added a comment -

          Thanks for the Report Peter - if someone creates a patch that fixes this I'd be happy to review it, otherwise there isn't currently any funding available for us to work on SCORM 2004.

          Show
          Dan Marsden added a comment - Thanks for the Report Peter - if someone creates a patch that fixes this I'd be happy to review it, otherwise there isn't currently any funding available for us to work on SCORM 2004.
          Hide
          S.H. added a comment -

          This problem also happens with Scorm 1.2. The underscore appears at line 317 of scorm_12.js.php.

          I haven't fount out why this happens.

          Show
          S.H. added a comment - This problem also happens with Scorm 1.2. The underscore appears at line 317 of scorm_12.js.php. I haven't fount out why this happens.
          Hide
          S.H. added a comment -

          Dan/Peter: Do you know why the dot will be changed to an underscore?
          It's probably fixable, but does it break something else?

          Show
          S.H. added a comment - Dan/Peter: Do you know why the dot will be changed to an underscore? It's probably fixable, but does it break something else?
          Hide
          Andrew Smith added a comment -

          Hi Dan/Peter, I'm not aware with any issues in scorm 1.2 as reported by S.H (the 1.2 javascript api should be using underscores) but the issue as originally reported is to do with the code which generates the _count values in scorm_reconstitute_array_element() in mod/scorm/locallib.php.

          Everywhere else in this function you test the API version before outputting lines of javascript, except for when you output the _count line for elements which contain sub-items.

          So for example, the current code produces:

          cmi.interactions_N1.objectives._count

          when it should produce

          cmi.interactions.N1.objectives._count

          This has been happening for a long time, it's not a 1.9.7 only issue. I've just had to make the patch again on a new install and thought it's about time I submitted a patch.

          Attached is a diff, showing a fix for the issue in Moodle 1.9.7+ (Build: 20100217)

          I'm sure this method of submitting a patch is inappropriate and I should be producing a proper patch against the head code in csv. But I haven't had an opportunity to read your policies for submitting patches.

          If you need me to resubmit this another way, please let me know.

          — locallib.php
          +++ (clipboard)
          @@ -1177,6 +1177,9 @@
          }
          if (count($matches) > 0 && $current != $matches[1]) {
          if ($count_sub > 0)

          { + if ($sversion == 'scorm_13') + echo ' '.$element_name.'.'.$current.'.'.$current_subelement.'._count = '.$count_sub.";\n"; + else echo ' '.$element_name.'_'.$current.'.'.$current_subelement.'._count = '.$count_sub.";\n"; }

          $current = $matches[1];

          Show
          Andrew Smith added a comment - Hi Dan/Peter, I'm not aware with any issues in scorm 1.2 as reported by S.H (the 1.2 javascript api should be using underscores) but the issue as originally reported is to do with the code which generates the _count values in scorm_reconstitute_array_element() in mod/scorm/locallib.php. Everywhere else in this function you test the API version before outputting lines of javascript, except for when you output the _count line for elements which contain sub-items. So for example, the current code produces: cmi.interactions_N1.objectives._count when it should produce cmi.interactions.N1.objectives._count This has been happening for a long time, it's not a 1.9.7 only issue. I've just had to make the patch again on a new install and thought it's about time I submitted a patch. Attached is a diff, showing a fix for the issue in Moodle 1.9.7+ (Build: 20100217) I'm sure this method of submitting a patch is inappropriate and I should be producing a proper patch against the head code in csv. But I haven't had an opportunity to read your policies for submitting patches. If you need me to resubmit this another way, please let me know. — locallib.php +++ (clipboard) @@ -1177,6 +1177,9 @@ } if (count($matches) > 0 && $current != $matches [1] ) { if ($count_sub > 0) { + if ($sversion == 'scorm_13') + echo ' '.$element_name.'.'.$current.'.'.$current_subelement.'._count = '.$count_sub.";\n"; + else echo ' '.$element_name.'_'.$current.'.'.$current_subelement.'._count = '.$count_sub.";\n"; } $current = $matches [1] ;
          Hide
          Andrew Smith added a comment -

          ok, well putting it in the comment didn't work, so here is the diff as a text file

          Show
          Andrew Smith added a comment - ok, well putting it in the comment didn't work, so here is the diff as a text file
          Hide
          Dan Marsden added a comment -

          Thanks Andrew, I'm pretty busy getting some patches ready for inclusion in 2.0 over the next 2 or 3 weeks, but I'll try to have a look at your patch sometime next month.

          thanks,

          Dan

          Show
          Dan Marsden added a comment - Thanks Andrew, I'm pretty busy getting some patches ready for inclusion in 2.0 over the next 2 or 3 weeks, but I'll try to have a look at your patch sometime next month. thanks, Dan
          Hide
          Andrew Smith added a comment -

          Just letting you know, this issue has changed line numbers after recent changes is CVS. It's currently on line 1173 in the 1.9.8 weekly available as of today's date.

          By the way it's the same issue you fixed on lines 1202 and 1229 (again these line numbers are according to this week's weekly).

          Show
          Andrew Smith added a comment - Just letting you know, this issue has changed line numbers after recent changes is CVS. It's currently on line 1173 in the 1.9.8 weekly available as of today's date. By the way it's the same issue you fixed on lines 1202 and 1229 (again these line numbers are according to this week's weekly).
          Hide
          Dan Marsden added a comment -

          Hi Andrew, thanks for the update on this one - I forgot there was a patch attached to this issue! - thanks for that! - I've pushed a fix into 1.9Stable and HEAD - would be great if you could test/provide feedback.

          It looks like this is the same as MDL-18202 - is that correct?

          Show
          Dan Marsden added a comment - Hi Andrew, thanks for the update on this one - I forgot there was a patch attached to this issue! - thanks for that! - I've pushed a fix into 1.9Stable and HEAD - would be great if you could test/provide feedback. It looks like this is the same as MDL-18202 - is that correct?

            People

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

              Dates

              • Created:
                Updated:
                Resolved: