Cheers,
Piers Harding.

http://cvs.moodle.org/moodle/mod/scorm/datamodels/scorm_12.js.php?r1=1.23&r2=1.24

thanks,

result is now proposed to be evaluated through the return value of StoreData; this return value is built with this logic below:

result = DoRequest(myRequest,"<?php p($CFG->wwwroot) ?>/mod/scorm/datamodel.php","id=<?php p($id) ?>&sesskey=<?php p($USER->sesskey) ?>"+datastring);
        results = String(result).split('\n');
        errorCode = results[1];
        return results[0];

Everything will work as expected (i.e. true or false) until the AJAX call will fail:

if (httpReq.status == 200) {
        //popupwin(url+"\n"+param+"\n"+httpReq.responseText);
        return httpReq.responseText;
    } else {
        return httpReq.status;
    }

so it could be IMHO better to code ''true if true, false otherwise'', e.g.:

@@ -199,13 +199,20 @@
                         setTimeout('top.document.location=top.next;',500);
                     }
                 }
+                <?php
+                    if (debugging('',DEBUG_DEVELOPER)) {
+                        echo 'LogAPICall("LMSFinish", "AJAXResult", result, 0);';
+                    }
+                ?>
+                result = ('true' == result ? 'true' : 'false');
                 <?php
                     if (debugging('',DEBUG_DEVELOPER)) {
                         //echo 'alert("Finished SCORM 1.2");';
+                        echo 'LogAPICall("LMSFinish", "result", result, 0);';
                         echo 'LogAPICall("LMSFinish", param, "", 0);';
                     }
                 ?>
-                return "true";
+                return result;
             } else {
                 errorCode = "301";
             }

Note: I've rearranged also the LogAPICall parameters order, according to the signature&logic found in scorm_12.js.php.
Note: we're interested in the fix of this issue also in 1.9 branch.

However, we are about to setup a new instlation of Moodle that will serve over 10,000 user starting in March and we are very concerned that the number of users experiencing this issue is going to climb rather quickly just by volume.

1. Moodle should correctly implement the LMSFinish('') return value 'cause of a specification that tells how to do that. That's the reason for filing this bug as well as the sense of my review proposal;
2. Content vendors should correctly react on a failure of an LMSFinish('') call (= having got "false" as the return value) by adopting a recovery strategy like e.g. try to re-call it after n seconds to mitigate e.g. network issues OR using sometime LMSCommit('').
   Those things above just to avoid false myths, see also Best practices for the use of Commit in SCORM content.

BTW, @Ron: if you're using Mastery Score in your packages, it could be possible that a failure in setting the cmi.core.lesson_status value could issue what described in MDL-21305.

If it is set to false, then errorCode should also be set to conform to SCORM standards. I recommend the following:

result = ('true' == result) ? 'true' : 'false';
errorCode = (result == 'true')? '0' : '101';

Thanks,
Matteo