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

SCORM session test for / detect network failure

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.12, 2.0.3, 2.3.3, 2.4.6, 2.5, 2.6
    • Fix Version/s: 2.7
    • Component/s: SCORM
    • Environment:
      All
    • Database:
      Any
    • Testing Instructions:
      Hide
      1. Add a scorm package
      2. View the package in the player (e.g. as a student would)
      3. Open the JS Console and keep an eye on the console
      4. Wait for 20-30 seconds
      5. Shut your web server down (e.g. service apache2 stop)
      6. Switch back to your browser - DO NOT REFRESH
      7. Wait 20-30 seconds
        • Confirm that an alert was shown
      8. Restart your web server - DO NOT REFRESH
      9. Wait 20-30 seconds
        • Confirm that the alert disappeared
        • Remove /lib/yui/build/moodle-core-checknet/assets/checknet.txt
      10. Wait 20-30 seconds
        • Confirm that an alert was shown
      11. Click the 'Ok' button
      12. Wait a few seconds
        • Confirm that an alert was shown again
      13. Place the file back
      14. Wait 20-30 seconds
        • Confirm that the alert disappeared
      Show
      Add a scorm package View the package in the player (e.g. as a student would) Open the JS Console and keep an eye on the console Wait for 20-30 seconds Shut your web server down (e.g. service apache2 stop) Switch back to your browser - DO NOT REFRESH Wait 20-30 seconds Confirm that an alert was shown Restart your web server - DO NOT REFRESH Wait 20-30 seconds Confirm that the alert disappeared Remove /lib/yui/build/moodle-core-checknet/assets/checknet.txt Wait 20-30 seconds Confirm that an alert was shown Click the 'Ok' button Wait a few seconds Confirm that an alert was shown again Place the file back Wait 20-30 seconds Confirm that the alert disappeared
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_27_STABLE
    • Pull Master Branch:
      MDL-28261-master

      Description

      One of the most common issues I deal with with clients is that their learners sometimes do not have learner progress recorded. The scorm packages that I see associated with this kind of issue are all the same in the way they behave: they initialize and send some data on initial load, and then do not send any data until the last page view of the scorm content. I suspect that somewhere in there the learner (on hotel wireless, traveling, not the best 'net connection, whatever) has network dropped, and when data tries to save, it is not saved, but the SCORM object does not tell them at any time that network connection has been lost. Some vendors create content that only calls LMSCommit() once in a single 20m session, just before LMSFinish()... and according to the SCORM standard this is not incorrect.

      It would be great if the Moodle scorm player could check the user's network connection at intervals, and notify the learner if network connection was intermittent or lost. This would preempt these issues with crappy vendor content. From what I can tell, if the network connection is lost, then LMSCommit() and other calls fail but do not return an error message. In my own testing, try/catch on these calls only catches in some browsers, not all.

      Based on my own research, I've been able to determine that it is possible to test network failure by injecting an image into the content page with a same-domain src. This test works in webkit and ff. (not in IE). I also tested the .isonline status, but that is not reliable either. So it seems there isn't any reliable cross-browser way of checking the learner's network connection using javascript.

      But I wanted to raise the issue here just in case there is a server-side way, that we could access from the scorm player code, of checking the network at intervals during an api session. I know this can add to bandwidth/load stuff, but it would be great, even as an option to turn on if a site had a lot of learners who weren't having their results saved.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            danmarsden Dan Marsden added a comment -

            sounds like a good idea - if you manage to convince one of your devs there to throw together a patch I'd be happy to review it for inclusion - otherwise I'll leave this in the queue and might get to it one day!

            Show
            danmarsden Dan Marsden added a comment - sounds like a good idea - if you manage to convince one of your devs there to throw together a patch I'd be happy to review it for inclusion - otherwise I'll leave this in the queue and might get to it one day!
            Hide
            agroshek Amy Groshek added a comment -

            I have not been able to recruit anyone to my cause here. But I would like to take a look at it when I have time and see if I can make some headway. A super-awesome thing we might do, if connection is down, is check for application cache support in the browser and if it is available, store interaction data in the app cache...

            Show
            agroshek Amy Groshek added a comment - I have not been able to recruit anyone to my cause here. But I would like to take a look at it when I have time and see if I can make some headway. A super-awesome thing we might do, if connection is down, is check for application cache support in the browser and if it is available, store interaction data in the app cache...
            Hide
            agroshek Amy Groshek added a comment -

            Hi Dan,

            I have a very simple proof of concept on this issue, available at my github fork: https://github.com/amygroshek/moodle
            Changes are addition of checknet.php and json obj checkNet added to player.php.

            Right now it's not tied to any SCORM player process, which might be best. I have no idea whether this is the best place to house the JavaScript, or whether simply adding another PHP file to the mod is a good approach.

            If you have time, let me know what you think.

            Show
            agroshek Amy Groshek added a comment - Hi Dan, I have a very simple proof of concept on this issue, available at my github fork: https://github.com/amygroshek/moodle Changes are addition of checknet.php and json obj checkNet added to player.php. Right now it's not tied to any SCORM player process, which might be best. I have no idea whether this is the best place to house the JavaScript, or whether simply adding another PHP file to the mod is a good approach. If you have time, let me know what you think.
            Hide
            danmarsden Dan Marsden added a comment -

            Hi Amy,

            great to hear! - the best way to manage changes in github is to keep the "master" branch (and other standard Moodle branches clean) - and create a new branch based on the original branch with your changes - that way you can use github to show a nice diff like this:
            https://github.com/danmarsden/moodle/compare/master...master_MDL-28335

            can you please strip out your changes onto a different branch so I can see a clean diff?

            thanks!

            Show
            danmarsden Dan Marsden added a comment - Hi Amy, great to hear! - the best way to manage changes in github is to keep the "master" branch (and other standard Moodle branches clean) - and create a new branch based on the original branch with your changes - that way you can use github to show a nice diff like this: https://github.com/danmarsden/moodle/compare/master...master_MDL-28335 can you please strip out your changes onto a different branch so I can see a clean diff? thanks!
            Hide
            agroshek Amy Groshek added a comment - - edited

            This is showing some changed lines that I didn't really change, but hopefully this is still helpful. If not I can redo it. https://github.com/amygroshek/moodle/compare/master...MDL-28261

            Show
            agroshek Amy Groshek added a comment - - edited This is showing some changed lines that I didn't really change, but hopefully this is still helpful. If not I can redo it. https://github.com/amygroshek/moodle/compare/master...MDL-28261
            Hide
            danmarsden Dan Marsden added a comment -

            Hi Amy - have you refined this at all since your last post? - a good place for those JS functions would be mod/scorm/module.js

            I also wonder if checknet.php should just include config.php and check to make sure user is still logged into Moodle? - thoughts?

            Show
            danmarsden Dan Marsden added a comment - Hi Amy - have you refined this at all since your last post? - a good place for those JS functions would be mod/scorm/module.js I also wonder if checknet.php should just include config.php and check to make sure user is still logged into Moodle? - thoughts?
            Hide
            agroshek Amy Groshek added a comment - - edited

            Hi Dan,

            That is just the kind of feedback I was hoping for! I haven't made any changes yet. Had no idea if I was on the right track.

            ==> a good place for those JS functions would be mod/scorm/module.js
            This should work fine provided that module.js is loaded on original load of the scorm player. It looks to me like that's the case.

            ==> I also wonder if checknet.php should just include config.php and check to make sure user is still logged into Moodle?
            I'm sure you know far more than me in this regard. The only restriction I see with checknet.php is that the check itself needs to be located in a PHP file that is not loaded from the server on initial load of the player, so that we are making a request back to the server when we call for it. Do you see checking for login as being in addition to checking connection server-side, or that we would just check for logged in state at that point?

            Would you like me to make these changes and present you with a fresh branch to look over?

            Show
            agroshek Amy Groshek added a comment - - edited Hi Dan, That is just the kind of feedback I was hoping for! I haven't made any changes yet. Had no idea if I was on the right track. ==> a good place for those JS functions would be mod/scorm/module.js This should work fine provided that module.js is loaded on original load of the scorm player. It looks to me like that's the case. ==> I also wonder if checknet.php should just include config.php and check to make sure user is still logged into Moodle? I'm sure you know far more than me in this regard. The only restriction I see with checknet.php is that the check itself needs to be located in a PHP file that is not loaded from the server on initial load of the player, so that we are making a request back to the server when we call for it. Do you see checking for login as being in addition to checking connection server-side, or that we would just check for logged in state at that point? Would you like me to make these changes and present you with a fresh branch to look over?
            Hide
            danmarsden Dan Marsden added a comment -

            I was thinking the login check could have the added benefit of keeping the user logged in for sites with a low login session time and SCORMs that can be quite long?

            I don't think the check you have inside checknet.php that does the fsockopen is needed as to load the checknet.php page, the user will have to have a connection to the server anyway - unless I'm missing something else?

            Show
            danmarsden Dan Marsden added a comment - I was thinking the login check could have the added benefit of keeping the user logged in for sites with a low login session time and SCORMs that can be quite long? I don't think the check you have inside checknet.php that does the fsockopen is needed as to load the checknet.php page, the user will have to have a connection to the server anyway - unless I'm missing something else?
            Hide
            agroshek Amy Groshek added a comment -

            Yes, that makes perfect sense. The fsockopen, as you say, isn't necessary, and just checks server connectivity, not client connectivity. The login check is a better idea.

            Show
            agroshek Amy Groshek added a comment - Yes, that makes perfect sense. The fsockopen, as you say, isn't necessary, and just checks server connectivity, not client connectivity. The login check is a better idea.
            Hide
            agroshek Amy Groshek added a comment - - edited

            Hi Dan,

            I moved the JS to module.js, and made changes to checknet.php.

            Show
            agroshek Amy Groshek added a comment - - edited Hi Dan, I moved the JS to module.js, and made changes to checknet.php.
            Hide
            agroshek Amy Groshek added a comment - - edited

            In response to the following inquiry I'm uploading the relevant files to this ticket, in case anyone wants to work with/improve them.

            https://moodle.org/mod/forum/discuss.php?d=218352

            checknet.php is a new file that is added to the SCORM mod dir. module.js contains a new object, M.mod_scorm.checkNet, and an init.

            In my tests, I found that using the YUI panel was not the best strategy because if the connection failed before YUI panel was loaded, the script would not be able to load YUI's panel mode either. Then there would be no alert, just a JS error to the page. After this version, I created a second version which used plain old alert() instead, and while this is not as pretty, it's important that the alert be as reliable as possible if it's to really address the issue at hand. Looking into this further. Looks like this is the final version, and contains the alert() instead of using YUI panel.

            Show
            agroshek Amy Groshek added a comment - - edited In response to the following inquiry I'm uploading the relevant files to this ticket, in case anyone wants to work with/improve them. https://moodle.org/mod/forum/discuss.php?d=218352 checknet.php is a new file that is added to the SCORM mod dir. module.js contains a new object, M.mod_scorm.checkNet, and an init. In my tests, I found that using the YUI panel was not the best strategy because if the connection failed before YUI panel was loaded, the script would not be able to load YUI's panel mode either. Then there would be no alert, just a JS error to the page. After this version, I created a second version which used plain old alert() instead, and while this is not as pretty, it's important that the alert be as reliable as possible if it's to really address the issue at hand. Looking into this further. Looks like this is the final version, and contains the alert() instead of using YUI panel.
            Hide
            danmarsden Dan Marsden added a comment -

            I forgot about this one - I'll take another look when I get back from leave in Jan - I can't remember if this code had the effect of keeping the user logged in or if it just checked to see if the user is logged in and threw a js warning if not? - If it keeps the user logged in we will probably need to add some form of control around the code.

            Show
            danmarsden Dan Marsden added a comment - I forgot about this one - I'll take another look when I get back from leave in Jan - I can't remember if this code had the effect of keeping the user logged in or if it just checked to see if the user is logged in and threw a js warning if not? - If it keeps the user logged in we will probably need to add some form of control around the code.
            Hide
            agroshek Amy Groshek added a comment - - edited

            Hi Dan,

            The JS calls a PHP file which does check logged in status.

            Cheers,
            A

            Show
            agroshek Amy Groshek added a comment - - edited Hi Dan, The JS calls a PHP file which does check logged in status. Cheers, A
            Hide
            ngomes Neil Gomes added a comment -

            Hello Amy and Dan,

            We are using MOODLE v2.3.2+ (Build: 20120927).

            We are experiencing a very similar issue. Some of our SCORM Training Modules are long and some of our learners can tend to step away from the training for long enough that their login to the LMS times out. We have set the MOODLE session timeout to 120 minutes, but have found this is still not a solution.

            The issue is that the SCORM module displays no indication of this timeout and so when the learner returns to the computer and resumes the training, they can do so. However, when they submit or close the module's page (it opens in a new window), none of their progress from before or after the login timeout is recorded.

            Could this solution that you, Amy, have proposed be what we need? If so, Dan, can we deploy this in our version of MOODLE? Amy, does the learner get a warning as soon as the logout is detected and would logging in again and then resuming retain their progress in the SCORM module and MOODLE?

            Any help with this would be greatly appreciated. This is turning out to be quite a problem for us and our learners.

            Thanks Amy and Dan. I look forward to hearing from you.

            Best wishes,

            Neil Gomes
            ngomes@health.usf.edu

            Show
            ngomes Neil Gomes added a comment - Hello Amy and Dan, We are using MOODLE v2.3.2+ (Build: 20120927). We are experiencing a very similar issue. Some of our SCORM Training Modules are long and some of our learners can tend to step away from the training for long enough that their login to the LMS times out. We have set the MOODLE session timeout to 120 minutes, but have found this is still not a solution. The issue is that the SCORM module displays no indication of this timeout and so when the learner returns to the computer and resumes the training, they can do so. However, when they submit or close the module's page (it opens in a new window), none of their progress from before or after the login timeout is recorded. Could this solution that you, Amy, have proposed be what we need? If so, Dan, can we deploy this in our version of MOODLE? Amy, does the learner get a warning as soon as the logout is detected and would logging in again and then resuming retain their progress in the SCORM module and MOODLE? Any help with this would be greatly appreciated. This is turning out to be quite a problem for us and our learners. Thanks Amy and Dan. I look forward to hearing from you. Best wishes, Neil Gomes ngomes@health.usf.edu
            Hide
            agroshek Amy Groshek added a comment -

            Hi Neil,

            Could this solution that you, Amy, have proposed be what we need?

            This will probably do what you need. The best way to find out is to test the branch out. The default interval the check runs at is every 5 seconds, so the notification will happen pretty soon after timeout.

            If so, Dan, can we deploy this in our version of MOODLE?

            You might need to rebase my branch off of the latest release, or cherry-pick it onto your own branch.

            Amy, does the learner get a warning as soon as the logout is detected

            Yes

            and would logging in again and then resuming retain their progress in the SCORM module and MOODLE

            The SCO is still responsible for saving learner progress as often as possible, and the LMS will only have saved up to the last LMSCommit(), but the learner is notified that they're no longer being tracked.

            Show
            agroshek Amy Groshek added a comment - Hi Neil, Could this solution that you, Amy, have proposed be what we need? This will probably do what you need. The best way to find out is to test the branch out. The default interval the check runs at is every 5 seconds, so the notification will happen pretty soon after timeout. If so, Dan, can we deploy this in our version of MOODLE? You might need to rebase my branch off of the latest release, or cherry-pick it onto your own branch. Amy, does the learner get a warning as soon as the logout is detected Yes and would logging in again and then resuming retain their progress in the SCORM module and MOODLE The SCO is still responsible for saving learner progress as often as possible, and the LMS will only have saved up to the last LMSCommit(), but the learner is notified that they're no longer being tracked.
            Hide
            ngomes Neil Gomes added a comment -

            Hi Amy,

            Thank you so very much for the quick response. I have sent your response to our MOODLE admin and asked if he could do this. I might have to come back to you if he has any questions. I hope that's okay. Thanks again.

            Best wishes,

            Neil

            Show
            ngomes Neil Gomes added a comment - Hi Amy, Thank you so very much for the quick response. I have sent your response to our MOODLE admin and asked if he could do this. I might have to come back to you if he has any questions. I hope that's okay. Thanks again. Best wishes, Neil
            Hide
            danmarsden Dan Marsden added a comment -

            I keep meaning to look at this - we can't use require_login() as it could have the effect of keeping the user logged in forever - we need to use isloggedin() instead.

            there's also a hardcoded lang in the js and I wonder if there's an easier way we can do it using YUI without all that code?

            Show
            danmarsden Dan Marsden added a comment - I keep meaning to look at this - we can't use require_login() as it could have the effect of keeping the user logged in forever - we need to use isloggedin() instead. there's also a hardcoded lang in the js and I wonder if there's an easier way we can do it using YUI without all that code?
            Hide
            agroshek Amy Groshek added a comment -

            Hi Dan Marsden,

            Simple to change to isloggedin(). With regard to the lang and using YUI, I would appreciate it if you could be more specific.

            If you mean using YUI panel for the alert, I tried that, and discovered that when the panel is instantiated, several additional assets are requested from the domain. If the network connection has been dropped, then these requests will obviously fail. So we would have to instantiate the panel undisplayed when the window was first loaded, which is the only time we know for sure we have a network connection, and do so even though for many users it would never be used. We could always do it that way if you feel that's the best way. It's just adding to the weight of the page load.

            Show
            agroshek Amy Groshek added a comment - Hi Dan Marsden , Simple to change to isloggedin(). With regard to the lang and using YUI, I would appreciate it if you could be more specific. If you mean using YUI panel for the alert, I tried that, and discovered that when the panel is instantiated, several additional assets are requested from the domain. If the network connection has been dropped, then these requests will obviously fail. So we would have to instantiate the panel undisplayed when the window was first loaded, which is the only time we know for sure we have a network connection, and do so even though for many users it would never be used. We could always do it that way if you feel that's the best way. It's just adding to the weight of the page load.
            Hide
            agroshek Amy Groshek added a comment -

            Hi Dan Marsden,

            I've updated the php file to use isloggedin() and altered the JS object to use a string from the lang pack (only in 2.4 for now). New diff for inspection: https://github.com/amygroshek/moodle/commit/6b9ccc657ec80d304a126e4f22b6a1cc7f0556de

            Show
            agroshek Amy Groshek added a comment - Hi Dan Marsden , I've updated the php file to use isloggedin() and altered the JS object to use a string from the lang pack (only in 2.4 for now). New diff for inspection: https://github.com/amygroshek/moodle/commit/6b9ccc657ec80d304a126e4f22b6a1cc7f0556de
            Hide
            danmarsden Dan Marsden added a comment -

            Thanks Amy - bouncing this up for peer review - I'm probably not the best person to review YUI3 code - I haven't done much YUI3 work yet.

            I also wonder if this is something that other modules might find useful? - will be interesting to hear feedback from HQ

            Show
            danmarsden Dan Marsden added a comment - Thanks Amy - bouncing this up for peer review - I'm probably not the best person to review YUI3 code - I haven't done much YUI3 work yet. I also wonder if this is something that other modules might find useful? - will be interesting to hear feedback from HQ
            Hide
            danmarsden Dan Marsden added a comment -

            Just had a chat with Andrew and he pointed me towards MDL-34498 which looks like it will resolve this at a higher level - also reminded me that by including config.php in the checknet.php we are modifying the session which could cause a user to be logged in indefinitely which is a security risk.

            Show
            danmarsden Dan Marsden added a comment - Just had a chat with Andrew and he pointed me towards MDL-34498 which looks like it will resolve this at a higher level - also reminded me that by including config.php in the checknet.php we are modifying the session which could cause a user to be logged in indefinitely which is a security risk.
            Hide
            rmeske Ron Meske added a comment -

            Hi Dan & Amy,

            The fix to warn of a session about to expire is definitely an improvement to help with timeouts. How will this function if a SCORM activity is opened in a new window?

            Will this also help when there is a poor network connection or it is completely lost. We find that with the proliferation of wireless hotspots, many people are now taking training away from work and home, in a coffee shop, on a train, etc. This has led to many people complaining that they took the course but it didn't save their progress. If LMSCommit was to at least verify whether or not it actually saved the data would go a long way to helping with this problem.

            If LMSCommit did indicate an error that the data didn't save, then we can add to our own code a way to save the data locally and inform the user of this fact having them exit at the time. At a subsequent launch our code would check for saved data, compare to what was loaded on launch, and then update with the most current version.

            Show
            rmeske Ron Meske added a comment - Hi Dan & Amy, The fix to warn of a session about to expire is definitely an improvement to help with timeouts. How will this function if a SCORM activity is opened in a new window? Will this also help when there is a poor network connection or it is completely lost. We find that with the proliferation of wireless hotspots, many people are now taking training away from work and home, in a coffee shop, on a train, etc. This has led to many people complaining that they took the course but it didn't save their progress. If LMSCommit was to at least verify whether or not it actually saved the data would go a long way to helping with this problem. If LMSCommit did indicate an error that the data didn't save, then we can add to our own code a way to save the data locally and inform the user of this fact having them exit at the time. At a subsequent launch our code would check for saved data, compare to what was loaded on launch, and then update with the most current version.
            Hide
            agroshek Amy Groshek added a comment - - edited

            Will this also help when there is a poor network connection or it is completely lost.

            Ron Meske This fix is designed to detect network failure outside of the functioning of LMSCommit. Independent JS tests against a PHP script. If the PHP script is inaccessible, the user is shown a JS alert. Because of security concerns, there is no support for this fix in the community and it's unlikely to be integrated, but you're welcome to use it, and I can rebase to more current versions if you like.

            Dan Marsden So if we don't require config.php in checknet.php we eliminate the security risk? Seems like a simple change. It doesn't look like MDL-34498 is going to be implemented. Of course then we're not checking loggedin... but this seems like a tautology, that we have to check logged in status, but we can't because we might keep the session active forever. Folks do find and use this patch.

            Show
            agroshek Amy Groshek added a comment - - edited Will this also help when there is a poor network connection or it is completely lost. Ron Meske This fix is designed to detect network failure outside of the functioning of LMSCommit. Independent JS tests against a PHP script. If the PHP script is inaccessible, the user is shown a JS alert. Because of security concerns, there is no support for this fix in the community and it's unlikely to be integrated, but you're welcome to use it, and I can rebase to more current versions if you like. Dan Marsden So if we don't require config.php in checknet.php we eliminate the security risk? Seems like a simple change. It doesn't look like MDL-34498 is going to be implemented. Of course then we're not checking loggedin... but this seems like a tautology, that we have to check logged in status, but we can't because we might keep the session active forever. Folks do find and use this patch.
            Hide
            agroshek Amy Groshek added a comment -

            Adding rebased branches. I've got two separate commits. This form is not allowing me to enter multiple diff urls.

            Show
            agroshek Amy Groshek added a comment - Adding rebased branches. I've got two separate commits. This form is not allowing me to enter multiple diff urls.
            Hide
            agroshek Amy Groshek added a comment -

            Dan Marsden I've revised so that the AJAX request goes to an empty HTML file rather than a PHP file. No security risks, and much faster than calling wwwroot. Of course we're not checking logged in status, but all the request gets back is a blank file, no config data, etc.

            Show
            agroshek Amy Groshek added a comment - Dan Marsden I've revised so that the AJAX request goes to an empty HTML file rather than a PHP file. No security risks, and much faster than calling wwwroot. Of course we're not checking logged in status, but all the request gets back is a blank file, no config data, etc.
            Hide
            danmarsden Dan Marsden added a comment -

            Thanks Amy - bouncing this up for peer review (this would be a master-only patch at this stage)

            Petr Skoda Andrew Nicols would be great if you guys could provide feedback here - especially around any security issues that might arise - are we safe checking an html file - is there a better way we could check the user is logged in without modifying the session?

            Show
            danmarsden Dan Marsden added a comment - Thanks Amy - bouncing this up for peer review (this would be a master-only patch at this stage) Petr Skoda Andrew Nicols would be great if you guys could provide feedback here - especially around any security issues that might arise - are we safe checking an html file - is there a better way we could check the user is logged in without modifying the session?
            Hide
            matteo Matteo Scaramuccia added a comment -

            Hi All,
            the user can't be tracked without being logged in, see https://github.com/moodle/moodle/blob/master/mod/scorm/datamodel.php#L51.
            Shortly, if we think that this network failure check must be unrelated with potential invalid session issues - which are actually something really different from a network failure! - , Amy's current PR is IMHO correct: if I could vote I'd vote for her current PR.
            The only improvement could be to hit e.g. 1 byte data/txt file to reduce the bandwidth impact of such checking - I'm really pedantic, I know - and avoiding some strange web server configurations like e.g. AddHandler application/x-httpd-php .html.

            Invalid session and session timeouts must IMHO be tracked by a failure of LMSCommit()/LMSFInish(), easily handled by the Content that should be the Owner of telling the user about that communication issue.

            Maybe MDL-34498 will take care of session timeouts in a different way helping the Content Tracking too but any invalid/timed-out session check must be IMHO optional OR implemented using a JS timeout, client side, to avoid an infinite user session.

            HTH,
            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - Hi All, the user can't be tracked without being logged in, see https://github.com/moodle/moodle/blob/master/mod/scorm/datamodel.php#L51 . Shortly, if we think that this network failure check must be unrelated with potential invalid session issues - which are actually something really different from a network failure ! - , Amy's current PR is IMHO correct: if I could vote I'd vote for her current PR. The only improvement could be to hit e.g. 1 byte data/txt file to reduce the bandwidth impact of such checking - I'm really pedantic, I know - and avoiding some strange web server configurations like e.g. AddHandler application/x-httpd-php .html . Invalid session and session timeouts must IMHO be tracked by a failure of LMSCommit() / LMSFInish() , easily handled by the Content that should be the Owner of telling the user about that communication issue. Maybe MDL-34498 will take care of session timeouts in a different way helping the Content Tracking too but any invalid/timed-out session check must be IMHO optional OR implemented using a JS timeout, client side, to avoid an infinite user session. HTH, Matteo
            Hide
            agroshek Amy Groshek added a comment -

            Matteo, good idea! I'm going to make that change.

            Show
            agroshek Amy Groshek added a comment - Matteo , good idea! I'm going to make that change.
            Hide
            danmarsden Dan Marsden added a comment -

            Thanks Matteo - Amy I'm 99% sure that checking an html file shouldn't have any security issues - it just would be nice if we could detect the user had logged out as well but I'm not sure we can do that safely - keen to hear from Petr/Andrew if they have ideas on how we could do it in Moodle as part of this but it's not vital. Thanks!

            Show
            danmarsden Dan Marsden added a comment - Thanks Matteo - Amy I'm 99% sure that checking an html file shouldn't have any security issues - it just would be nice if we could detect the user had logged out as well but I'm not sure we can do that safely - keen to hear from Petr/Andrew if they have ideas on how we could do it in Moodle as part of this but it's not vital. Thanks!
            Hide
            danmarsden Dan Marsden added a comment -

            Juan Leyva Is there anything we need to be mindful here related to the Mobile App? - I imagine that a large number of mobile devices are likely to drop connection/go offline at some point.

            Show
            danmarsden Dan Marsden added a comment - Juan Leyva Is there anything we need to be mindful here related to the Mobile App? - I imagine that a large number of mobile devices are likely to drop connection/go offline at some point.
            Hide
            rmeske Ron Meske added a comment -

            I know I am late to the conversation, but isn't there a simpler way to handle a network disconnect for LMSCommit and LMSFinish?

            What we see as the issue is that LMSCommit doesn't return an error if the network connection is lost. To find out why I opened IE's debugger and found that a JavaScript error is thrown when StoreData calls DoRequest and executes httpReq.send(param). As this error is not captured, an error is never returned to LMSCommit and an error is never returned to our course.

            Show
            rmeske Ron Meske added a comment - I know I am late to the conversation, but isn't there a simpler way to handle a network disconnect for LMSCommit and LMSFinish? What we see as the issue is that LMSCommit doesn't return an error if the network connection is lost. To find out why I opened IE's debugger and found that a JavaScript error is thrown when StoreData calls DoRequest and executes httpReq.send(param). As this error is not captured, an error is never returned to LMSCommit and an error is never returned to our course.
            Hide
            agroshek Amy Groshek added a comment -

            Hi Ron Meske, in fact there isn't. If the network connection is dropped, then there is no API, and LMSCommit() can't return an error through an API that has been dropped. If the SCO itself was checking not only for error codes but for no response, then yes. But that means we have to depend upon the content developers to do so. That is, in effect, what we're doing now. The solution is certainly convoluted, but the API isn't an option.

            Show
            agroshek Amy Groshek added a comment - Hi Ron Meske , in fact there isn't. If the network connection is dropped, then there is no API, and LMSCommit() can't return an error through an API that has been dropped. If the SCO itself was checking not only for error codes but for no response, then yes. But that means we have to depend upon the content developers to do so. That is, in effect, what we're doing now. The solution is certainly convoluted, but the API isn't an option.
            Hide
            danmarsden Dan Marsden added a comment -

            thanks Ron - that does sound familiar - if you haven't created a bug for that already can you please create one and include the information on the javascript error that occurs?

            Show
            danmarsden Dan Marsden added a comment - thanks Ron - that does sound familiar - if you haven't created a bug for that already can you please create one and include the information on the javascript error that occurs?
            Hide
            rmeske Ron Meske added a comment - - edited

            Hi Amy, As a developer I can wrap my call to LMSCommit in a try/catch to capture this condition. But for others that use off-the-self authoring tools, could the httpReq.send(param) be wrapped in a try catch statement and if there is an error return false?

            Hi Dan, The error is "XMLHttpRequest: Network Error 0x2ee7, Could not complete the operation due to error 00002ee7."Should this be a separate bug report? If so, I will create it.

            Show
            rmeske Ron Meske added a comment - - edited Hi Amy, As a developer I can wrap my call to LMSCommit in a try/catch to capture this condition. But for others that use off-the-self authoring tools, could the httpReq.send(param) be wrapped in a try catch statement and if there is an error return false? Hi Dan, The error is "XMLHttpRequest: Network Error 0x2ee7, Could not complete the operation due to error 00002ee7."Should this be a separate bug report? If so, I will create it.
            Hide
            agroshek Amy Groshek added a comment -

            I see what you're saying, Ron Meske. It seems like a good idea to add a try/catch there, or, better yet, use Y.io and a timeout. But there are content packages that call LMSCommit() only once for the whole activity, at the completion of an hour's worth of content, just before calling LMSFinish(). In which case the user is thinking their progress is saved all that time when it isn't. If we're at the caprices of the developer's failed LMSCommit() to detect network failure, then we're still depending on the developer to actually make that call at regular intervals and the solution is only as good as the content developer.

            Show
            agroshek Amy Groshek added a comment - I see what you're saying, Ron Meske . It seems like a good idea to add a try/catch there, or, better yet, use Y.io and a timeout. But there are content packages that call LMSCommit() only once for the whole activity, at the completion of an hour's worth of content, just before calling LMSFinish(). In which case the user is thinking their progress is saved all that time when it isn't. If we're at the caprices of the developer's failed LMSCommit() to detect network failure, then we're still depending on the developer to actually make that call at regular intervals and the solution is only as good as the content developer.
            Hide
            danmarsden Dan Marsden added a comment -

            Ron - definitely a new bug report please - makes sense to try/catch and return a proper error rather than a quiet failure.

            Show
            danmarsden Dan Marsden added a comment - Ron - definitely a new bug report please - makes sense to try/catch and return a proper error rather than a quiet failure.
            Hide
            jleyva Juan Leyva added a comment -

            Hi Dan Marsden,

            Tony Blundell and Dominic Orme are working in some MOBILE issues related to this, MOBILE-446 and MOBILE-448 maybe they are interested in this issue

            Regards

            Show
            jleyva Juan Leyva added a comment - Hi Dan Marsden , Tony Blundell and Dominic Orme are working in some MOBILE issues related to this, MOBILE-446 and MOBILE-448 maybe they are interested in this issue Regards
            Hide
            kineodominico Dominic Orme added a comment - - edited

            Hi Amy,

            I've had a look at your JavaScript, just a couple of comments.
            1. Don't pass a string as the first argument of setInterval. Although it works, and offers no security issues, it has to be reparsed, so is therefore slower. The line goes from:

            var t = setInterval('M.mod_scorm.checknet.check()', this.interval);

            to

            var t = setInterval(M.mod_scorm.checknet.check, this.interval);

            (StackOverflow setInterval)

            2. In your throwpanel function you do

            var $this = this

            and then proceed to use $this. However, as the code never changes context throughout that function, so $this is redundant, and you could just use 'this' throughout.
            (StackOverflow $this)

            Neither change is a must-have, however earlier you were asking for feedback.

            Show
            kineodominico Dominic Orme added a comment - - edited Hi Amy, I've had a look at your JavaScript, just a couple of comments. 1. Don't pass a string as the first argument of setInterval. Although it works, and offers no security issues, it has to be reparsed, so is therefore slower. The line goes from: var t = setInterval('M.mod_scorm.checknet.check()', this.interval); to var t = setInterval(M.mod_scorm.checknet.check, this.interval); ( StackOverflow setInterval ) 2. In your throwpanel function you do var $this = this and then proceed to use $this. However, as the code never changes context throughout that function, so $this is redundant, and you could just use 'this' throughout. ( StackOverflow $this ) Neither change is a must-have, however earlier you were asking for feedback.
            Hide
            jleyva Juan Leyva added a comment -

            Not sure if I'm missing something but the current code is still pointing to checknet.html instead checknet.txt

             url: M.cfg.wwwroot + '/mod/scorm/checknet.html',

            Show
            jleyva Juan Leyva added a comment - Not sure if I'm missing something but the current code is still pointing to checknet.html instead checknet.txt url: M.cfg.wwwroot + '/mod/scorm/checknet.html' ,
            Hide
            agroshek Amy Groshek added a comment -

            [Juan Leyva Good catch. Dominic Orme, if I set up the interval using the string, then checknet.check() has the correct scope. If I call using a function pointer, then checknet.check() does not have the correct scope. Further reading from your own reference: http://stackoverflow.com/a/5801700/590107 . The other option is to use the function pointer, but reset $this within checknet.check(). Which looks pretty garish:

               check: function() {
                    var $this = M.mod_scorm.checknet;
                    YUI().use("io-base", function(Y) {
                        // Check quality of response data
                        function successCheck(id, o) {

            If you have a different suggestion, I'm open to it, but I'm also sure Andrew can weigh in. For now I'm sticking with calling from the global scope.

            Show
            agroshek Amy Groshek added a comment - [ Juan Leyva Good catch. Dominic Orme , if I set up the interval using the string, then checknet.check() has the correct scope. If I call using a function pointer, then checknet.check() does not have the correct scope. Further reading from your own reference: http://stackoverflow.com/a/5801700/590107 . The other option is to use the function pointer, but reset $this within checknet.check(). Which looks pretty garish: check: function() { var $this = M.mod_scorm.checknet; YUI().use("io-base", function(Y) { // Check quality of response data function successCheck(id, o) { If you have a different suggestion, I'm open to it, but I'm also sure Andrew can weigh in. For now I'm sticking with calling from the global scope.
            Hide
            kineodominico Dominic Orme added a comment -

            I'm not sure whether this needs to be performance optimised ( even just a little ), but if it does then doing

            Boolean(o.responseText);

            is horribly inefficient and should be replaced with a double negation, although pretty much anything else is faster anyway.
            Benchmarks here: Boolean Conversion

            Show
            kineodominico Dominic Orme added a comment - I'm not sure whether this needs to be performance optimised ( even just a little ), but if it does then doing Boolean(o.responseText); is horribly inefficient and should be replaced with a double negation, although pretty much anything else is faster anyway. Benchmarks here: Boolean Conversion
            Hide
            kineodominico Dominic Orme added a comment -

            Reading through the code, I see you're using 'alert'. I read above, fair enough, useful library isn't loading fast enough/isn't available. I'm still curious why you went with alert though? It can't be styled, and multiple alerts are frequently flagged by modern browsers and discarded. I can see the reason for a modal dialog box though.

            The code below is a library (and hopefully browser) agnostic popover that is made of a backing div (#popover) and the actual popover (#actualbox) with rudimentary styling to go in a css file somewhere. It's not modal although it could be made to be.( if #popover covered the visible page, and body had overflow:hidden on )

            function createPopover(content, button) {
                if (content.trim() == "") { 
                    content = "Non Blank Content Required"; 
                }
                if (button.trim() == "") { 
                    button = "OK"; 
                }
             
                var popover = document.createElement("div");
                popover.id = 'popover';
                var actualbox = document.createElement("div");
                actualbox.id = 'actualbox';
                var textElement = document.createElement('p');
                var button = document.createElement('button');
             
                textElement.textContent = content;
                button.textContent = button;
                button.onclick = function() { document.getElementById('popover').remove(); }
             
                actualbox.appendChild(textElement);
                actualbox.appendChild(button);
                popover.appendChild(actualbox);
             
                document.body.appendChild(popover);
            };

            The css would need to be along the lines of:

            #popover {
                position:absolute;
                z-index:2;
                width:100%;
            }
             
            #actualbox {
                width:300px;
                height:150px;
                background-color:#ffffff;
                border:1px solid #000000;
                margin:0 auto;
            }

            The browser modal alert is just so... ugly.

            Show
            kineodominico Dominic Orme added a comment - Reading through the code, I see you're using 'alert'. I read above, fair enough, useful library isn't loading fast enough/isn't available. I'm still curious why you went with alert though? It can't be styled, and multiple alerts are frequently flagged by modern browsers and discarded. I can see the reason for a modal dialog box though. The code below is a library (and hopefully browser) agnostic popover that is made of a backing div (#popover) and the actual popover (#actualbox) with rudimentary styling to go in a css file somewhere. It's not modal although it could be made to be.( if #popover covered the visible page, and body had overflow:hidden on ) function createPopover(content, button) { if (content.trim() == "") { content = "Non Blank Content Required"; } if (button.trim() == "") { button = "OK"; }   var popover = document.createElement("div"); popover.id = 'popover'; var actualbox = document.createElement("div"); actualbox.id = 'actualbox'; var textElement = document.createElement('p'); var button = document.createElement('button');   textElement.textContent = content; button.textContent = button; button.onclick = function() { document.getElementById('popover').remove(); }   actualbox.appendChild(textElement); actualbox.appendChild(button); popover.appendChild(actualbox);   document.body.appendChild(popover); }; The css would need to be along the lines of: #popover { position:absolute; z-index:2; width:100%; }   #actualbox { width:300px; height:150px; background-color:#ffffff; border:1px solid #000000; margin:0 auto; } The browser modal alert is just so... ugly.
            Hide
            agroshek Amy Groshek added a comment -

            Hi Dominic Orme. Thanks for the recommendation. If we did implement something I'm sure HQ would want to use YUI. I left the object with a panel placeholder so we can utilize YUI's panel later if necessary, but it seems like overkill to me to load CSS and JS and instantiate a warning modal panel when most users won't ever see it. It has to happen when the page loads, or else the connection may be lost when we attempt to instantiate it and requisite CSS and JS files for YUI modal will be missing. But loading those files in advance of something that may never happen will impact overall load time for every SCORM player on every site. Dan Marsden, Andrew Nicols, and others might have opinions.

            There shouldn't be multiple alerts, just one (unless there's some wacky timing issue and two different kinds of failures one after the other, before the interval is cleared on the first, and in that case just two alerts). It'll be an easy alteration to add later, if it's wanted. My goal at this point is to get a solution implemented, in order to lighten the load for our support team and others doing same. The alert is generally-understood convention that every device is going to handle in a manner appropriate to that device, without a bunch of styling and tweaking.

            Show
            agroshek Amy Groshek added a comment - Hi Dominic Orme . Thanks for the recommendation. If we did implement something I'm sure HQ would want to use YUI. I left the object with a panel placeholder so we can utilize YUI's panel later if necessary, but it seems like overkill to me to load CSS and JS and instantiate a warning modal panel when most users won't ever see it. It has to happen when the page loads, or else the connection may be lost when we attempt to instantiate it and requisite CSS and JS files for YUI modal will be missing. But loading those files in advance of something that may never happen will impact overall load time for every SCORM player on every site. Dan Marsden , Andrew Nicols , and others might have opinions. There shouldn't be multiple alerts, just one (unless there's some wacky timing issue and two different kinds of failures one after the other, before the interval is cleared on the first, and in that case just two alerts). It'll be an easy alteration to add later, if it's wanted. My goal at this point is to get a solution implemented, in order to lighten the load for our support team and others doing same. The alert is generally-understood convention that every device is going to handle in a manner appropriate to that device, without a bunch of styling and tweaking.
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Hi Amy Groshek,

            Thanks for working on this - I've just had a quick peek at the history of the ticket. We should really try and get a solution for this in to 2.7.

            Tim Hunt has also been working on a similar problem for the Quiz in MDL-42504. His change is now up for Integration Review, but perhaps we should move it into a more generic YUI class that can be applied across Moodle.

            Tim's solution to the Panel vs. alert() debate was to use neither and to have a hidden element which is unhidden when required. You're right when you say that Panel brings in certain assets and that this could cause an issue, but equally alert() is also not a great choice because of it's modality.

            Tim Hunt, any thoughts on this? I believe it should be trivial to convert it to a standalone module which takes a few arguments (title, message, timeout, URL) and fires off a few events. This would give us a consistent, reusable solution across Moodle.

            Andrew

            Show
            dobedobedoh Andrew Nicols added a comment - Hi Amy Groshek , Thanks for working on this - I've just had a quick peek at the history of the ticket. We should really try and get a solution for this in to 2.7. Tim Hunt has also been working on a similar problem for the Quiz in MDL-42504 . His change is now up for Integration Review, but perhaps we should move it into a more generic YUI class that can be applied across Moodle. Tim's solution to the Panel vs. alert() debate was to use neither and to have a hidden element which is unhidden when required. You're right when you say that Panel brings in certain assets and that this could cause an issue, but equally alert() is also not a great choice because of it's modality. Tim Hunt , any thoughts on this? I believe it should be trivial to convert it to a standalone module which takes a few arguments (title, message, timeout, URL) and fires off a few events. This would give us a consistent, reusable solution across Moodle. Andrew
            Hide
            agroshek Amy Groshek added a comment -

            Andrew Nicols I'm interested to hear Tim's ideas. SCORM just needs the network check, not monitoring of form fields... It's also not clear to me how a single hidden/displayed element is going to work for SCORM. Maybe Dan Marsden has some ideas. I can see where that type of solution is more intuitive for the quiz, but with SCORM there are two possible display contexts (original page and pop-up), and there is so much variability in both in terms of dimensions. In many cases the content already occupies the full screen and we can't count on having 100 vertical pixels to expand a warning in the page. In those cases a modal dialog or alert makes most sense. Once again, I defer to Dan Marsden.

            Show
            agroshek Amy Groshek added a comment - Andrew Nicols I'm interested to hear Tim's ideas. SCORM just needs the network check, not monitoring of form fields... It's also not clear to me how a single hidden/displayed element is going to work for SCORM. Maybe Dan Marsden has some ideas. I can see where that type of solution is more intuitive for the quiz, but with SCORM there are two possible display contexts (original page and pop-up), and there is so much variability in both in terms of dimensions. In many cases the content already occupies the full screen and we can't count on having 100 vertical pixels to expand a warning in the page. In those cases a modal dialog or alert makes most sense. Once again, I defer to Dan Marsden .
            Hide
            timhunt Tim Hunt added a comment -

            I am still hoping MDL-42504 will get into 2.6, so please let us not disturb that possibility now, even if we plan something better for 2.7.

            Any, the "monitoring of form field" stuff is separate from the notification. That quiz auto-save was done in 2.5, and then based on that, notification that you had gone off-line was easy to add, so I did.

            Andrew's point is that the two things are really separate, and so the 'autosave quiz' and 'notify if internet/login lost' are really separate things, and the notify if offline bit is potentially very reusable, and so it should be separated out.

            However, it makes sense to use the existing Ajax calls that the JS is making anyway to verify that the user is still online. No point adding extra server load for this, unless there is no other option.

            Note that the alert I implemented floats over the top of the rest of the page, but is non-modal so that you can copy-and-paste what you have typed into the page if you want to save it.

            Show
            timhunt Tim Hunt added a comment - I am still hoping MDL-42504 will get into 2.6, so please let us not disturb that possibility now, even if we plan something better for 2.7. Any, the "monitoring of form field" stuff is separate from the notification. That quiz auto-save was done in 2.5, and then based on that, notification that you had gone off-line was easy to add, so I did. Andrew's point is that the two things are really separate, and so the 'autosave quiz' and 'notify if internet/login lost' are really separate things, and the notify if offline bit is potentially very reusable, and so it should be separated out. However, it makes sense to use the existing Ajax calls that the JS is making anyway to verify that the user is still online. No point adding extra server load for this, unless there is no other option. Note that the alert I implemented floats over the top of the rest of the page, but is non-modal so that you can copy-and-paste what you have typed into the page if you want to save it.
            Hide
            agroshek Amy Groshek added a comment - - edited

            Andrew Nicols This is one of the most common SCORM support issues we have. Worse, it's a diagnosis of exclusion, which means it's too high-level for unskilled support staff to make and is therefore quite costly. It seems like Tim Hunt really wants to get his solution into Moodle and working now. I'm in the same boat. Couldn't we get these improvements into 2.6 and plan to abstract out to a more modular solution for 2.7?

            Others can weigh in. I don't really have a stake in what Tim's doing with the quiz. But I'm pretty strongly of the opinion that for SCORM we want to prevent additional interaction with the content if the network connection is lost, with something like an alert or modal, because of the variability of layout options as well as the JS resizing of the player size with screen size. We wouldn't want to take any chance that that warning is not in view when displayed. Even if it is, the alert or modal prevents the user from continuing and forces them to investigate the issue. (Moreover, can't you still copy text from a form field after closing a modal or alert? It's only page reload that's going to do away with what's typed.)

            In terms of tying the network connection checks to existing API calls, that puts us with the same catch-22 as discussed above, where we are only checking the network as often as the SCORM content is calling through the API. For some content, these calls only are made twice: on launch and on exit, with an hour or more in between. So while that makes sense for the quiz, it doesn't necessarily make sense for SCORM, where the fix is needed in part because some SCORM content makes API calls so infrequently. Dan Marsden might have an opinion about that.

            Show
            agroshek Amy Groshek added a comment - - edited Andrew Nicols This is one of the most common SCORM support issues we have. Worse, it's a diagnosis of exclusion, which means it's too high-level for unskilled support staff to make and is therefore quite costly. It seems like Tim Hunt really wants to get his solution into Moodle and working now. I'm in the same boat. Couldn't we get these improvements into 2.6 and plan to abstract out to a more modular solution for 2.7? Others can weigh in. I don't really have a stake in what Tim's doing with the quiz. But I'm pretty strongly of the opinion that for SCORM we want to prevent additional interaction with the content if the network connection is lost, with something like an alert or modal, because of the variability of layout options as well as the JS resizing of the player size with screen size. We wouldn't want to take any chance that that warning is not in view when displayed. Even if it is, the alert or modal prevents the user from continuing and forces them to investigate the issue. (Moreover, can't you still copy text from a form field after closing a modal or alert? It's only page reload that's going to do away with what's typed.) In terms of tying the network connection checks to existing API calls, that puts us with the same catch-22 as discussed above, where we are only checking the network as often as the SCORM content is calling through the API. For some content, these calls only are made twice: on launch and on exit, with an hour or more in between. So while that makes sense for the quiz, it doesn't necessarily make sense for SCORM, where the fix is needed in part because some SCORM content makes API calls so infrequently. Dan Marsden might have an opinion about that.
            Hide
            danmarsden Dan Marsden added a comment -

            IMO this has missed the boat for inclusion in 2.6 before it's released - If integrators really have heaps of time on their hands and want to integrate it's fine by me but I suspect that is not the case.

            I think there may be a good justification for this to land in stable branches though so we may not need to wait until 2.7 - I'm personally not too worried about the arguments between using alert/modal/non-modal at this stage - as long as the approach works ok on multiple devices. I would be ok with this going in on the first version as an alert and then allow others (or Amy) to improve it later if required - but I'll defer to the integrators on whether this will be allowed or not.

            A future improvement for 2.7 (or higher) could be to create a core api function to test for network failure and then migrate quiz/scorm to use that solution.

            Show
            danmarsden Dan Marsden added a comment - IMO this has missed the boat for inclusion in 2.6 before it's released - If integrators really have heaps of time on their hands and want to integrate it's fine by me but I suspect that is not the case. I think there may be a good justification for this to land in stable branches though so we may not need to wait until 2.7 - I'm personally not too worried about the arguments between using alert/modal/non-modal at this stage - as long as the approach works ok on multiple devices. I would be ok with this going in on the first version as an alert and then allow others (or Amy) to improve it later if required - but I'll defer to the integrators on whether this will be allowed or not. A future improvement for 2.7 (or higher) could be to create a core api function to test for network failure and then migrate quiz/scorm to use that solution.
            Hide
            agroshek Amy Groshek added a comment -

            Dan Marsden Core api function sounds great.

            Show
            agroshek Amy Groshek added a comment - Dan Marsden Core api function sounds great.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            FYI: I'm going to perform a test with this issue (as far as it's missing the remote repository field).

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - FYI: I'm going to perform a test with this issue (as far as it's missing the remote repository field).
            Hide
            cibot CiBoT added a comment -

            Results for MDL-28261 (https://tracker.moodle.org/browse/MDL-28261)

            • Error: the repository field is empty. Nothing was checked.
            Show
            cibot CiBoT added a comment - Results for MDL-28261 ( https://tracker.moodle.org/browse/MDL-28261 ) Error: the repository field is empty. Nothing was checked.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Done, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Done, thanks!
            Hide
            cibot CiBoT added a comment -

            Results for MDL-28261

            • Error: the repository field is empty. Nothing was checked.
            Show
            cibot CiBoT added a comment - Results for MDL-28261 Error: the repository field is empty. Nothing was checked.
            Hide
            marina Marina Glancy added a comment -

            Hi Amy, thanks for working on such a voted issue.
            From the quick glance the code has some errors against JS guidelines. For example, we never use function "alert". Would be really nice if our JS experts can review it, maybe Andrew Nicols ?

            Show
            marina Marina Glancy added a comment - Hi Amy, thanks for working on such a voted issue. From the quick glance the code has some errors against JS guidelines. For example, we never use function "alert". Would be really nice if our JS experts can review it, maybe Andrew Nicols ?
            Hide
            agroshek Amy Groshek added a comment -

            Hi Marina Glancy. I haven't brought the code up to the new standards because there's been no action on the issue. If there's an interest, I'm more than happy to make revisions.

            Show
            agroshek Amy Groshek added a comment - Hi Marina Glancy . I haven't brought the code up to the new standards because there's been no action on the issue. If there's an interest, I'm more than happy to make revisions.
            Hide
            rmeske Ron Meske added a comment -

            Hi Amy Groshek, I would really like to see this fixed. Our clients only use SCORM courses, and like you indicated, this is the type of support issue that is not easy to diagnose and causes the most frustration of those users who encounter it.

            Show
            rmeske Ron Meske added a comment - Hi Amy Groshek, I would really like to see this fixed. Our clients only use SCORM courses, and like you indicated, this is the type of support issue that is not easy to diagnose and causes the most frustration of those users who encounter it.
            Hide
            marina Marina Glancy added a comment -

            Amy, the status of the issue is "Waiting for peer review" that's why I started looking at it.
            There are many votes and people who would like to see it so would be really nice if you could finish it. Thanks a lot!

            Show
            marina Marina Glancy added a comment - Amy, the status of the issue is "Waiting for peer review" that's why I started looking at it. There are many votes and people who would like to see it so would be really nice if you could finish it. Thanks a lot!
            Hide
            dobedobedoh Andrew Nicols added a comment -

            In the interests of progressing this, I'm going to ruthlessly pinch it off you if you don't mind Amy.

            Show
            dobedobedoh Andrew Nicols added a comment - In the interests of progressing this, I'm going to ruthlessly pinch it off you if you don't mind Amy.
            Hide
            agroshek Amy Groshek added a comment -

            Andrew Nicols Pinch away. It was clearly never going to make it through peer review with my name on it anyway.

            Marina Glancy If you take the time to read the comments, you'll see I've been working on this issue for 3 years. Every year I get a bunch of vague comments, I make several changes, and then nothing happens. If HQ has suddenly developed an enthusiasm for the issue, then thank God. I have long since come to the conclusion that there wasn't much more I could do to advance the issue.

            Show
            agroshek Amy Groshek added a comment - Andrew Nicols Pinch away. It was clearly never going to make it through peer review with my name on it anyway. Marina Glancy If you take the time to read the comments, you'll see I've been working on this issue for 3 years. Every year I get a bunch of vague comments, I make several changes, and then nothing happens. If HQ has suddenly developed an enthusiasm for the issue, then thank God. I have long since come to the conclusion that there wasn't much more I could do to advance the issue.
            Hide
            danmarsden Dan Marsden added a comment -

            Thanks Andrew! - I haven't performed a peer review but really great to see this happening!

            Show
            danmarsden Dan Marsden added a comment - Thanks Andrew! - I haven't performed a peer review but really great to see this happening!
            Hide
            skodak Petr Skoda added a comment - - edited

            Thanks everybody! pushing for integration

            Show
            skodak Petr Skoda added a comment - - edited Thanks everybody! pushing for integration
            Hide
            cibot CiBoT added a comment -

            Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

            Show
            cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Two little points:

            • I'm asking Helen Foster to take a look to the the strings introduced by this. Will add extra commits if decided to amend any string.
            • Should we consider backporting this? Sounds like a real possibility, isn't it?

            Integrating, master only... thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Two little points: I'm asking Helen Foster to take a look to the the strings introduced by this. Will add extra commits if decided to amend any string. Should we consider backporting this? Sounds like a real possibility, isn't it? Integrating, master only... thanks!
            Hide
            abgreeve Adrian Greeve added a comment -

            Tested on the master integration branch.
            Testing steps were easy to follow. Thank you
            Everything worked as described.
            Test passed.

            Show
            abgreeve Adrian Greeve added a comment - Tested on the master integration branch. Testing steps were easy to follow. Thank you Everything worked as described. Test passed.
            Hide
            matteo Matteo Scaramuccia added a comment -

            That's a great news!
            If I'd vote, here is my +2 to back-port it on both 2.5 and 2.6: as already told, it will help the end user SCORM experience, even in "understanding" some obscure tracking issues.

            TIA,
            Matteo

            Show
            matteo Matteo Scaramuccia added a comment - That's a great news! If I'd vote, here is my +2 to back-port it on both 2.5 and 2.6 : as already told, it will help the end user SCORM experience, even in "understanding" some obscure tracking issues. TIA, Matteo
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Eloy Lafuente (stronk7), I agree - it would be nice to backport. I've raised MDL-44913 in accordance with the Backport policy

            Show
            dobedobedoh Andrew Nicols added a comment - Eloy Lafuente (stronk7) , I agree - it would be nice to backport. I've raised MDL-44913 in accordance with the Backport policy
            Hide
            marina Marina Glancy added a comment -

            Thanks for your hard work. Your code is now part of Moodle.

            Show
            marina Marina Glancy added a comment - Thanks for your hard work. Your code is now part of Moodle.

              People

              • Votes:
                11 Vote for this issue
                Watchers:
                19 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/May/14