Details

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

      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.

        Gliffy Diagrams

        1. locallibScorm13_countPatch.txt
          0.5 kB
          Andrew Smith

          Issue Links

            Activity

            Hide
            danmarsden 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
            danmarsden 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
            shuyg 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
            shuyg 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
            shuyg 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
            shuyg 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
            asmith 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
            asmith 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
            asmith 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
            asmith 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
            danmarsden 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
            danmarsden 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
            asmith 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
            asmith 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
            danmarsden 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
            danmarsden 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:
                  Fix Release Date:
                  8/Jun/10