Moodle
  1. Moodle
  2. MDL-27837

Unmask Checkboxes Don't Work As Expected With More Than One Enrolment Key

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.2
    • Fix Version/s: 2.1.1, 2.2
    • Component/s: Enrolments
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      Create a course with two or more self-enrolment options with enrolment keys. As a student who isn't already enroled in the course, attempt to enrol yourself. You should see multiple enrolment key options. Each one should have its own Unmask checkbox next to it and clicking each checkbox should unmask the corresponding key entry.

      Show
      Create a course with two or more self-enrolment options with enrolment keys. As a student who isn't already enroled in the course, attempt to enrol yourself. You should see multiple enrolment key options. Each one should have its own Unmask checkbox next to it and clicking each checkbox should unmask the corresponding key entry.
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      wip-mdl-27837
    • Rank:
      17477

      Description

      When a course has more than one self-enrolment type with enrolment keys, the "Unmask" checkboxes for keys after the first one do not show up in the correct place and don't unmask the field. See attached screenshot of a course with two self-enrolment types, each with its own key. Instead of having one Unmask checkbox next to each enrolment key field, both checkboxes appear next to the first key field. Also, the first checkbox unmasks the first key but the second checkbox does nothing--it does not unmask either key.

      In the class definition of enrol_self_enrol_form, $mform->addElement() is passed the string "enrolpassword" which is used to generate the names and IDs of the fields and divs. This is fine with one enrolment type but with more than one, all the options have the same names and IDs. The Unmask checkboxes are inserted via Javascript into a div with ID: id_enrolpasswordunmaskdiv. There are two divs with that ID on the page so all the Unmask checkboxes show up for the first enrolment key, as seen in the screenshot.

      Unique IDs would prevent these unexpected behaviors when there is more than one enrolment key. Perhaps using the heading as part of the string would be helpful. I tried that and it puts the checkboxes in the proper places but changing the string in the addElement() call changes the field name in addition to the div ID and the unknown name breaks the form submission. So that will need to be worked out somehow too.

        Issue Links

          Activity

          Hide
          Michael Blake added a comment -

          This issues has been reported as causing problems for a MP. Please give it priority.

          Show
          Michael Blake added a comment - This issues has been reported as causing problems for a MP. Please give it priority.
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this.

          This will be given priority attention following the release of Moodle 2.1.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. This will be given priority attention following the release of Moodle 2.1.
          Hide
          Rajesh Taneja added a comment -

          As part of this fix, password_unmask has been re-written.
          Previous implementation doesn't allow more then one password_unmask fields on one page. Also, it was not using YUI framework.

          Show
          Rajesh Taneja added a comment - As part of this fix, password_unmask has been re-written. Previous implementation doesn't allow more then one password_unmask fields on one page. Also, it was not using YUI framework.
          Hide
          Rossiani Wijaya added a comment -

          Hi Raj,

          This looks great.
          My +1 to submit it for integration.

          Show
          Rossiani Wijaya added a comment - Hi Raj, This looks great. My +1 to submit it for integration.
          Hide
          Sam Hemelryk added a comment -

          Thanks Raj, this has been integrate now.
          However it has only been integrated to master and MOODLE_21_STABLE, I'll update the fix version shortly to reflect this.
          The reason for this is we don't make API changes to stable branches normally. In the case of MOODLE_21_STABLE this API change is safe and as such I've chosen to integrate it, however 20_STABLE is now truly stable.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Raj, this has been integrate now. However it has only been integrated to master and MOODLE_21_STABLE, I'll update the fix version shortly to reflect this. The reason for this is we don't make API changes to stable branches normally. In the case of MOODLE_21_STABLE this API change is safe and as such I've chosen to integrate it, however 20_STABLE is now truly stable. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          There was originally a 20_STABLE fix for this:
          Branch: wip-mdl-27837-MOODLE_20_STABLE
          Diff: https://github.com/rajeshtaneja/moodle/compare/MOODLE_20_STABLE...wip-mdl-27837-MOODLE_20_STABLE

          Show
          Sam Hemelryk added a comment - There was originally a 20_STABLE fix for this: Branch: wip-mdl-27837-MOODLE_20_STABLE Diff: https://github.com/rajeshtaneja/moodle/compare/MOODLE_20_STABLE...wip-mdl-27837-MOODLE_20_STABLE
          Hide
          Petr Škoda added a comment -

          Tested, works as described. Big thanks!

          Show
          Petr Škoda added a comment - Tested, works as described. Big thanks!
          Hide
          geoff robinson added a comment -

          Hi

          I have a similar problem with 2.0.2 will be upgrading, but thought I'd patch it anyway but the link is broken. Is this patch still available? Thanks if anyone can let me know.

          Geoff

          Show
          geoff robinson added a comment - Hi I have a similar problem with 2.0.2 will be upgrading, but thought I'd patch it anyway but the link is broken. Is this patch still available? Thanks if anyone can let me know. Geoff
          Hide
          Petr Škoda added a comment -

          Hello, you can search for MDL-27837 inside commit messages in your local git checkout.

          Show
          Petr Škoda added a comment - Hello, you can search for MDL-27837 inside commit messages in your local git checkout.

            People

            • Votes:
              16 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: