Moodle
  1. Moodle
  2. MDL-25757

Duplicate CMI values not showing in report (SCORM Module)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.9.10, 2.0, 2.0.4, 2.1.1, 2.2
    • Fix Version/s: 1.9.15, 2.0.6, 2.1.3
    • Component/s: SCORM
    • Labels:
    • Environment:
      Tested under windows using XAMPP and web host. All databases were MySQL.
    • Testing Instructions:
      Hide

      Please, test this under all branches!

      Turn the SCORM debugger on so you can manually pass values into SCORM:

      In the text field for "select data model element to get or set" enter:
      cmi.interactions_0.id
      in the value to set field enter:
      SC_0200_a
      Hit the LMSSetValue() button

      In the text field for "select data model element to get or set" enter:
      cmi.interactions_0.result
      in the value to set field enter:
      wrong
      Hit the LMSSetValue() button

      In the text field for "select data model element to get or set" enter:
      cmi.interactions_1.id
      in the value to set field enter:
      SC_0210_a
      Hit the LMSSetValue() button

      In the text field for "select data model element to get or set" enter:
      cmi.interactions_1.result
      in the value to set field enter:
      wrong
      Hit the LMSSetValue() button

      now hit the LMSCommit() button

      • now check the results of the above

      go to the reports page of the SCORM - in the basic report it lists all the users who have submitted data and the number of "attempts" - (note attempt number is NOT the number of times a user has entered a SCORM) - click on the attempt number and on the following page click on "track details" - Check to make sure The "id" and "result" sub-elements will are written for interaction 0 and interaction 1.

      Show
      Please, test this under all branches! Turn the SCORM debugger on so you can manually pass values into SCORM: In the text field for "select data model element to get or set" enter: cmi.interactions_0.id in the value to set field enter: SC_0200_a Hit the LMSSetValue() button In the text field for "select data model element to get or set" enter: cmi.interactions_0.result in the value to set field enter: wrong Hit the LMSSetValue() button In the text field for "select data model element to get or set" enter: cmi.interactions_1.id in the value to set field enter: SC_0210_a Hit the LMSSetValue() button In the text field for "select data model element to get or set" enter: cmi.interactions_1.result in the value to set field enter: wrong Hit the LMSSetValue() button now hit the LMSCommit() button now check the results of the above go to the reports page of the SCORM - in the basic report it lists all the users who have submitted data and the number of "attempts" - (note attempt number is NOT the number of times a user has entered a SCORM) - click on the attempt number and on the following page click on "track details" - Check to make sure The "id" and "result" sub-elements will are written for interaction 0 and interaction 1.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull Master Branch:
      master_MDL-25757
    • Rank:
      15175

      Description

      Requested reminder for Dan Marsden from forum issue:

      http://moodle.org/mod/forum/discuss.php?d=164735

      1. MDL-25757.patch
        3 kB
        Evan Irving-Pease
      2. MDL-25757-commented.patch
        5 kB
        Evan Irving-Pease

        Issue Links

          Activity

          Hide
          Matteo Scaramuccia added a comment -

          Hi All,
          it could be related with the patch proposal done in MDL-21761.
          Lacking of spare time now, I hope to take a look at it next week... not sure about both relation and spare time.

          Show
          Matteo Scaramuccia added a comment - Hi All, it could be related with the patch proposal done in MDL-21761 . Lacking of spare time now, I hope to take a look at it next week... not sure about both relation and spare time.
          Hide
          Dan Marsden added a comment -

          thanks Matteo - I've got my head down doing 2.0 conversion work and won't get to it until the new year!

          Show
          Dan Marsden added a comment - thanks Matteo - I've got my head down doing 2.0 conversion work and won't get to it until the new year!
          Hide
          Dan Marsden added a comment -

          has anyone managed to track this one down?

          Show
          Dan Marsden added a comment - has anyone managed to track this one down?
          Hide
          Howard Miller added a comment -

          Having seen this myself, let me know if there's anything I can do to assist/test.

          Show
          Howard Miller added a comment - Having seen this myself, let me know if there's anything I can do to assist/test.
          Hide
          Derick Turner added a comment - - edited

          Just run through this with the patch from MDL-21761 taken out and the issue where the multiple interactions only have the first recorded fully is no longer happening. Looking at the code in the scorm_12.js_php file for the CollectData() function, there were two instances where the default value was being set. The patch only had one defined, so I changed it to be the same as in patch MDL-21761 and everything still looked to be working as I would expect.

                                      if ((typeof eval('datamodel["'+elementmodel+'"].defaultvalue')) != "undefined") {
                                          if (eval('datamodel["'+elementmodel+'"].defaultvalue') != data[property] || eval('typeof(datamodel["'+elementmodel+'"].defaultvalue)') != typeof(data[property])) {
                                              datastring += elementstring;
                                              eval('datamodel["'+elementmodel+'"].defaultvalue=data[property];');
                                          }
                                      } else {
                                          datastring += elementstring;
                                         // eval('datamodel["'+elementmodel+'"].defaultvalue=data[property];');
                                      }
          

          All of the interactions are now being recorded in the mdl_scorm_scoes_track database. As I don't know what the original issue was for this patch, I cannot check to see if this will re-introduce it.

          Show
          Derick Turner added a comment - - edited Just run through this with the patch from MDL-21761 taken out and the issue where the multiple interactions only have the first recorded fully is no longer happening. Looking at the code in the scorm_12.js_php file for the CollectData() function, there were two instances where the default value was being set. The patch only had one defined, so I changed it to be the same as in patch MDL-21761 and everything still looked to be working as I would expect. if ((typeof eval('datamodel[ "'+elementmodel+'" ].defaultvalue')) != "undefined" ) { if (eval('datamodel[ "'+elementmodel+'" ].defaultvalue') != data[property] || eval('typeof(datamodel[ "'+elementmodel+'" ].defaultvalue)') != typeof(data[property])) { datastring += elementstring; eval('datamodel[ "'+elementmodel+'" ].defaultvalue=data[property];'); } } else { datastring += elementstring; // eval('datamodel[ "'+elementmodel+'" ].defaultvalue=data[property];'); } All of the interactions are now being recorded in the mdl_scorm_scoes_track database. As I don't know what the original issue was for this patch, I cannot check to see if this will re-introduce it.
          Hide
          Matteo Scaramuccia added a comment -

          It's not related with the patch proposal in MDL-21761 but with the related commit:

          $ git diff b6ec0e79401db7^..b6ec0e79401db7
          

          or https://github.com/moodle/moodle/commit/b6ec0e79401db7.
          Derick comment is in the direction of promoting the patch proposal against the final commit.

          I can create some test cases using the Diagnostic SCO for both MDL-25757 and MDL-21761 but... not in the next weeks due to the lacking of spare time.

          Show
          Matteo Scaramuccia added a comment - It's not related with the patch proposal in MDL-21761 but with the related commit: $ git diff b6ec0e79401db7^..b6ec0e79401db7 or https://github.com/moodle/moodle/commit/b6ec0e79401db7 . Derick comment is in the direction of promoting the patch proposal against the final commit. I can create some test cases using the Diagnostic SCO for both MDL-25757 and MDL-21761 but... not in the next weeks due to the lacking of spare time.
          Hide
          Evan Irving-Pease added a comment -

          How's this issue progressing? At present Moodle is not SCORM 1.2 compliant becuase of this issue, and considing that it's been a known issue for 8 months now it would be great to see this fixed.

          Show
          Evan Irving-Pease added a comment - How's this issue progressing? At present Moodle is not SCORM 1.2 compliant becuase of this issue, and considing that it's been a known issue for 8 months now it would be great to see this fixed.
          Hide
          Dan Marsden added a comment -

          Hi Evan,

          it's in the list - just a matter of finding the time to do it - 95% of my SCORM work is done as a volunteer - so sometimes it can take a bit of time...

          I'll definitely have this resolved before 2.2 release - but that's still a while off - it will happen earlier if someone else provides a patch that solves it and doesn't cause any other issues - so it needs someone to spend time doing a lot of QA/testing.

          Show
          Dan Marsden added a comment - Hi Evan, it's in the list - just a matter of finding the time to do it - 95% of my SCORM work is done as a volunteer - so sometimes it can take a bit of time... I'll definitely have this resolved before 2.2 release - but that's still a while off - it will happen earlier if someone else provides a patch that solves it and doesn't cause any other issues - so it needs someone to spend time doing a lot of QA/testing.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This has been integrated, thanks! Testing feedback will be welcome!

          Show
          Eloy Lafuente (stronk7) added a comment - This has been integrated, thanks! Testing feedback will be welcome!
          Hide
          Tim Parham added a comment -

          What level of effort is required for the testing? I'd like to see the issue resolved and would be willing to help, if I can spare enough time.

          Show
          Tim Parham added a comment - What level of effort is required for the testing? I'd like to see the issue resolved and would be willing to help, if I can spare enough time.
          Hide
          Dan Marsden added a comment -

          Hi Tim,

          here's the info on the testing process:
          http://docs.moodle.org/dev/Testing

          basically this involves verifying that the patch has fixed the issue - as you need to look at the db to verify the patch has fixed the issue you will need to install a new version of Moodle based on the integration branch mentioned on the testing page.

          thanks!

          Show
          Dan Marsden added a comment - Hi Tim, here's the info on the testing process: http://docs.moodle.org/dev/Testing basically this involves verifying that the patch has fixed the issue - as you need to look at the db to verify the patch has fixed the issue you will need to install a new version of Moodle based on the integration branch mentioned on the testing page. thanks!
          Hide
          Evan Irving-Pease added a comment -

          Hi Dan,

          It's great to see some activity on this issue, thanks a lot.

          Unfortunatley I don't believe that the patch you've submitted for testing will fully solve the problem.

          By removing the eval call in /mod/scorm/datamodels/scorm_12.js.php on line 581 you've only fixed the bug for data model elements that don't have a default value.

          Your patch will solve the issue for elements like 'cmi.interactions.n.result' because it doesn't define a default value, but it will not work for elements like 'cmi.objectives.n.score.raw' that does define a default value (even though it's defined to be the empty string).

          Consequently you also need to remove the corresponding eval function call on line 577. Please see the patch I uploaded to MDL-28332.

          To reproduce the bug with your patch applied:

          LMSInitialize("", "")
          LMSSetValue("cmi.objectives_0.id", "objective_a")
          LMSSetValue("cmi.objectives_0.score.raw", "1")
          LMSSetValue("cmi.objectives_1.id", "objective_b")
          LMSSetValue("cmi.objectives_1.score.raw", "1")
          Commit("", "")

          The raw score for 'objective_b' is not saved.

          Regards,
          Evan.

          Show
          Evan Irving-Pease added a comment - Hi Dan, It's great to see some activity on this issue, thanks a lot. Unfortunatley I don't believe that the patch you've submitted for testing will fully solve the problem. By removing the eval call in /mod/scorm/datamodels/scorm_12.js.php on line 581 you've only fixed the bug for data model elements that don't have a default value. Your patch will solve the issue for elements like 'cmi.interactions.n.result' because it doesn't define a default value, but it will not work for elements like 'cmi.objectives.n.score.raw' that does define a default value (even though it's defined to be the empty string). Consequently you also need to remove the corresponding eval function call on line 577. Please see the patch I uploaded to MDL-28332 . To reproduce the bug with your patch applied: LMSInitialize("", "") LMSSetValue("cmi.objectives_0.id", "objective_a") LMSSetValue("cmi.objectives_0.score.raw", "1") LMSSetValue("cmi.objectives_1.id", "objective_b") LMSSetValue("cmi.objectives_1.score.raw", "1") Commit("", "") The raw score for 'objective_b' is not saved. Regards, Evan.
          Hide
          Dan Marsden added a comment -

          arg... ok... well problem with reverting that is we end up with the bug reported on MDL-21761 - so if anyone else wants to volunteer their time to suggest a fix for this that doesn't cause MDL-21761 to be a problem feel free! - I'm unlikely to have time to do it this week..

          NOTE TO INTEGRATOR/TESTERS - this bug should still pass and be included in the integration as it still provides a partial fix - thanks.

          Show
          Dan Marsden added a comment - arg... ok... well problem with reverting that is we end up with the bug reported on MDL-21761 - so if anyone else wants to volunteer their time to suggest a fix for this that doesn't cause MDL-21761 to be a problem feel free! - I'm unlikely to have time to do it this week.. NOTE TO INTEGRATOR/TESTERS - this bug should still pass and be included in the integration as it still provides a partial fix - thanks.
          Hide
          Evan Irving-Pease added a comment -

          Hmmm... arg indeed!

          I hadn't considered the problem of trying to reset a value back to its default, where that default has an actual value because the user is resuming an attempt.

          So to summarise the problem in this issue:

          When the code identifies during a call to LMSCommit() that a non-default value has been set for a field it updates the internal reference for that field's default value to match the value that has been set. Consequently when LMSCommit() is called again it only sends values that have change since the last call.

          However the object used by the API to store the default values (datamodel) uses the 'n' style notation (e.g. cmi.interactions.n.result) and not the number style notation (e.g. cmi.interactions.1.result) so all data model elements with an "n" in their name are considered the same. If you then try and commit the same value for two elements with the same name but different values for "n" it considers the value for the second commit to be for the same element and as the value for the first commit, and as the values are equal it doesn't save the second commit.

          Consequently what needs to happen is for the datamodel object to track changes for "n" sytle elements by explicitly referencing the actual value of "n" (e.g. datamodel[cmi.objectives_0.score.raw]).

          This is achieved by checking if an index exists in the da

          Please see the revised patch which I've attached, which fixes this issue without breaking the fix for MDL-21761

          Regards,
          Evan.

          Show
          Evan Irving-Pease added a comment - Hmmm... arg indeed! I hadn't considered the problem of trying to reset a value back to its default, where that default has an actual value because the user is resuming an attempt. So to summarise the problem in this issue: When the code identifies during a call to LMSCommit() that a non-default value has been set for a field it updates the internal reference for that field's default value to match the value that has been set. Consequently when LMSCommit() is called again it only sends values that have change since the last call. However the object used by the API to store the default values (datamodel) uses the 'n' style notation (e.g. cmi.interactions.n.result) and not the number style notation (e.g. cmi.interactions.1.result) so all data model elements with an "n" in their name are considered the same. If you then try and commit the same value for two elements with the same name but different values for "n" it considers the value for the second commit to be for the same element and as the value for the first commit, and as the values are equal it doesn't save the second commit. Consequently what needs to happen is for the datamodel object to track changes for "n" sytle elements by explicitly referencing the actual value of "n" (e.g. datamodel [cmi.objectives_0.score.raw] ). This is achieved by checking if an index exists in the da Please see the revised patch which I've attached, which fixes this issue without breaking the fix for MDL-21761 Regards, Evan.
          Hide
          Dan Marsden added a comment -

          Hi Evan - thanks for the patch! - I'm going to get this bug rejected for this week and try to review it again next week..

          Ideally I'd like a few others to test your patch before I commit it too - Matteo/Howard?

          thanks!

          Dan

          Show
          Dan Marsden added a comment - Hi Evan - thanks for the patch! - I'm going to get this bug rejected for this week and try to review it again next week.. Ideally I'd like a few others to test your patch before I commit it too - Matteo/Howard? thanks! Dan
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm rejecting this by Dan's request.

          I've reverted the commits in the 4 branches...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I'm rejecting this by Dan's request. I've reverted the commits in the 4 branches...ciao
          Hide
          David Kennedy added a comment -

          Any update on this issue?

          We just realized we are hit with this issue (fairly majorly, if you ask me). I'm shocked to see it's been a known issue for 10 months with no resolution.

          Show
          David Kennedy added a comment - Any update on this issue? We just realized we are hit with this issue (fairly majorly, if you ask me). I'm shocked to see it's been a known issue for 10 months with no resolution.
          Hide
          Dan Marsden added a comment -

          You do know that most of us volunteer our time right?

          but a patch has been provided and I've asked for more people to test it and confirm it fixes the problem and doesn't introduce other issues - no-one has done that yet! - perhaps you are volunteering to test?

          Show
          Dan Marsden added a comment - You do know that most of us volunteer our time right? but a patch has been provided and I've asked for more people to test it and confirm it fixes the problem and doesn't introduce other issues - no-one has done that yet! - perhaps you are volunteering to test?
          Hide
          David Kennedy added a comment -

          Volunteers or not, for a product the size and scope of Moodle to have a major known issue that (per the comments) keeps getting kicked down the road for almost a year yet still advertises itself as SCORM compliant is unacceptable in my book. Sorry if that offends you.

          I will test this and report back. I don't have much choice...this is a showstopper and has already resulted in lost data.

          Show
          David Kennedy added a comment - Volunteers or not, for a product the size and scope of Moodle to have a major known issue that (per the comments) keeps getting kicked down the road for almost a year yet still advertises itself as SCORM compliant is unacceptable in my book. Sorry if that offends you. I will test this and report back. I don't have much choice...this is a showstopper and has already resulted in lost data.
          Hide
          Russell Coombe added a comment -

          But without volunteers, both testing and building, a product the size and scope of Moodle would either not see the light of day or cost the earth. BTW, It advertises 'support' for SCORM 1.2 and compliant for version 1.9.5

          Show
          Russell Coombe added a comment - But without volunteers, both testing and building, a product the size and scope of Moodle would either not see the light of day or cost the earth. BTW, It advertises 'support' for SCORM 1.2 and compliant for version 1.9.5
          Hide
          David Kennedy added a comment -

          I'm not arguing the need for volunteers. However, I also don't accept "volunteer" as an equivalent to immunity.

          Today, I set up a copy of our production server, installed 1.9.14, and finally installed this patch. Our SCORM courses appear to run without issue. Without the patch, I was not getting, say, interactions/responses past the first one where there were several of the same choice in a row. With it, I now see all details. I don't notice any other issues.

          I don't know much about the conflicting issue 21761 mentioned above, so I cannot vouch for that one. However, it does appear to resolve 25757 with no adverse effects noted.

          Show
          David Kennedy added a comment - I'm not arguing the need for volunteers. However, I also don't accept "volunteer" as an equivalent to immunity. Today, I set up a copy of our production server, installed 1.9.14, and finally installed this patch. Our SCORM courses appear to run without issue. Without the patch, I was not getting, say, interactions/responses past the first one where there were several of the same choice in a row. With it, I now see all details. I don't notice any other issues. I don't know much about the conflicting issue 21761 mentioned above, so I cannot vouch for that one. However, it does appear to resolve 25757 with no adverse effects noted.
          Hide
          Dan Marsden added a comment -

          thanks for providing info on your test results - I'll take another look at this in the next few weeks and look at pushing it into the latest code.

          FYI - SCORM in Moodle 1.9Stable and 2.0Stable passes all the ADL SCORM 1.2 tests for compliance(at least they did last time I ran them) - as Russell mentioned the only version that is "certified" is Moodle 1.9.5 - we don't submit each version to ADL for certification. So - afaik the "advertisment" you are referring to is correct.

          I don't think anyone is asking for immunity - I know I'm not and as "Official SCORM maintainer" I know many areas of SCORM need a massive amount of work - I've even managed to secure small amounts of funding to work on specific areas but there's still a lot more to go. I have spent a massive amount of volunteer time on SCORM and I don't really care if you think the amount of time I have to contribute is unacceptable, especially on a bug that I have asked for people to test and no-one has responded. In fact - I get more e-mails/correspondence about the lack of SCORM 2004 support than I have had anyone talk about this particular bug.

          I have a great appreciation to all the volunteers in the community that help - without them SCORM would not be a feature in Moodle at all. We've also had 2 GSOC students working on SCORM recently - one of which was focusing on a way for us to automate the ADL tests so we can ensure each weekly release doesn't introduce any regressions causing the ADL tests to fail.

          In any case - if you really want full SCORM certification/compliance in Moodle you are welcome to install Rusticis SCORM cloud which works well with Moodle and has full certification.

          Thanks for your contribution.

          Show
          Dan Marsden added a comment - thanks for providing info on your test results - I'll take another look at this in the next few weeks and look at pushing it into the latest code. FYI - SCORM in Moodle 1.9Stable and 2.0Stable passes all the ADL SCORM 1.2 tests for compliance(at least they did last time I ran them) - as Russell mentioned the only version that is "certified" is Moodle 1.9.5 - we don't submit each version to ADL for certification. So - afaik the "advertisment" you are referring to is correct. I don't think anyone is asking for immunity - I know I'm not and as "Official SCORM maintainer" I know many areas of SCORM need a massive amount of work - I've even managed to secure small amounts of funding to work on specific areas but there's still a lot more to go. I have spent a massive amount of volunteer time on SCORM and I don't really care if you think the amount of time I have to contribute is unacceptable, especially on a bug that I have asked for people to test and no-one has responded. In fact - I get more e-mails/correspondence about the lack of SCORM 2004 support than I have had anyone talk about this particular bug. I have a great appreciation to all the volunteers in the community that help - without them SCORM would not be a feature in Moodle at all. We've also had 2 GSOC students working on SCORM recently - one of which was focusing on a way for us to automate the ADL tests so we can ensure each weekly release doesn't introduce any regressions causing the ADL tests to fail. In any case - if you really want full SCORM certification/compliance in Moodle you are welcome to install Rusticis SCORM cloud which works well with Moodle and has full certification. Thanks for your contribution.
          Hide
          Evan Irving-Pease added a comment -

          Hi David,

          I can appreciate your frustration with this bug. I was was pretty alarmed myself when I discovered that we'd been losing data on one of our production sites. However, the lack of votes for this issue suggests that there really aren't a lot of affected people out there. Aside from you and I, no one is really clamouring for this to get fixed.

          As someone who benefits from Moodle's open source license the best way to contribute back to the community is to create patches for bugs like this (if you're able) or to test patches offered by others.

          To that end, your testing is appreciated. Having made the patch I'd like to see it integrated, but without volunteer testers that will not happen.

          Regards,
          Evan.

          Show
          Evan Irving-Pease added a comment - Hi David, I can appreciate your frustration with this bug. I was was pretty alarmed myself when I discovered that we'd been losing data on one of our production sites. However, the lack of votes for this issue suggests that there really aren't a lot of affected people out there. Aside from you and I, no one is really clamouring for this to get fixed. As someone who benefits from Moodle's open source license the best way to contribute back to the community is to create patches for bugs like this (if you're able) or to test patches offered by others. To that end, your testing is appreciated. Having made the patch I'd like to see it integrated, but without volunteer testers that will not happen. Regards, Evan.
          Hide
          David Kennedy added a comment -

          This morning, I applied 1.9.14 as well as this patch to our production Moodle sites. Naturally, I'll be watching this closely over the next several days for signs of problems. If I see anything, I'll report back.

          Show
          David Kennedy added a comment - This morning, I applied 1.9.14 as well as this patch to our production Moodle sites. Naturally, I'll be watching this closely over the next several days for signs of problems. If I see anything, I'll report back.
          Hide
          Dan Marsden added a comment -

          I think I'm mainly happy with this patch - only issue is that it really needs a few more in-line comments in the code to explain what it's doing - Evan - any chance you would like to do this? - the more comments the better! - also, if you'd like to get full credit for the patch rather than just in the comment of the commit you could do this:

          create a new git branch (ideally based on Moodle's master branch) somewhere that I can see - (github.com is good) - then commit the fix to that branch as a single commit - I can then cherry-pick/merge your change and keep the "author" tag as yourself.

          thanks,

          Show
          Dan Marsden added a comment - I think I'm mainly happy with this patch - only issue is that it really needs a few more in-line comments in the code to explain what it's doing - Evan - any chance you would like to do this? - the more comments the better! - also, if you'd like to get full credit for the patch rather than just in the comment of the commit you could do this: create a new git branch (ideally based on Moodle's master branch) somewhere that I can see - (github.com is good) - then commit the fix to that branch as a single commit - I can then cherry-pick/merge your change and keep the "author" tag as yourself. thanks,
          Hide
          Dan Marsden added a comment -

          Evan - do you want to push this into a git repo so you get credit for the commits? - otherwise I'll do it later this week myself.

          Show
          Dan Marsden added a comment - Evan - do you want to push this into a git repo so you get credit for the commits? - otherwise I'll do it later this week myself.
          Hide
          Evan Irving-Pease added a comment -

          Hi Dan, nah I don't care about the credit. Just happy to be of help. I was planning on adding those comments you requested but I've been off sick and then returned snowed under with other work. Will try to get to it today.

          Show
          Evan Irving-Pease added a comment - Hi Dan, nah I don't care about the credit. Just happy to be of help. I was planning on adding those comments you requested but I've been off sick and then returned snowed under with other work. Will try to get to it today.
          Hide
          Dan Marsden added a comment -

          cool - if you could add comments that would be awesome! - will submit for integration as soon as that comes through!

          thanks!

          Show
          Dan Marsden added a comment - cool - if you could add comments that would be awesome! - will submit for integration as soon as that comes through! thanks!
          Hide
          Evan Irving-Pease added a comment -

          Added commented patch.

          Show
          Evan Irving-Pease added a comment - Added commented patch.
          Hide
          Dan Marsden added a comment -

          NOTE TO INTEGRATOR - surprisingly the patch on master cherry-picks cleanly to all branches (including 1.9) - feel free to cherry-pick if easier.

          Show
          Dan Marsden added a comment - NOTE TO INTEGRATOR - surprisingly the patch on master cherry-picks cleanly to all branches (including 1.9) - feel free to cherry-pick if easier.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (19, 20, 21 and master) by cherry-picking, thanks Dan, Evan @ Co!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (19, 20, 21 and master) by cherry-picking, thanks Dan, Evan @ Co!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          (PS: Note that 1.9 is officially unsupported since some months ago. I just integrated this there because it seemed important for some people. Generally we won't be backporting anything, but security fixes, to 1.9).

          Show
          Eloy Lafuente (stronk7) added a comment - (PS: Note that 1.9 is officially unsupported since some months ago. I just integrated this there because it seemed important for some people. Generally we won't be backporting anything, but security fixes, to 1.9).
          Hide
          Ankit Agarwal added a comment -

          Working as expected
          Thanks

          Show
          Ankit Agarwal added a comment - Working as expected Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And this has landed upstream, just on time for the upcoming new releases week. Thanks for it!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - And this has landed upstream, just on time for the upcoming new releases week. Thanks for it! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: