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

QuickForm insertElementBefore shows a warning if there are multiple elements without a name

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.13, 2.0.4, 2.1.1
    • Fix Version/s: 2.1.6, 2.2.3
    • Component/s: Forms Library
    • Labels:
    • Testing Instructions:
      Hide

      At the end of course/edit_form.php function 'definition()' add the following line:
      $mform->insertElementBefore($mform->createElement('static', 'testing', 'This is a test', 'The test text'), 'idnumber');

      The element appears as expected, but there is also the following error message:
      Notice: Undefined index: in [folder-removed]/lib/pear/HTML/QuickForm.php on line 679
      Warning: array_search() expects parameter 2 to be array, null given in[folder-removed]/lib/pear/HTML/QuickForm.php on line 679

      Show
      At the end of course/edit_form.php function 'definition()' add the following line: $mform->insertElementBefore($mform->createElement('static', 'testing', 'This is a test', 'The test text'), 'idnumber'); The element appears as expected, but there is also the following error message: Notice: Undefined index: in [folder-removed] /lib/pear/HTML/QuickForm.php on line 679 Warning: array_search() expects parameter 2 to be array, null given in [folder-removed] /lib/pear/HTML/QuickForm.php on line 679
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-29147_insertElementBefore_warning

      Description

      When adding an element, the duplicates array is not updated if the element has no name.
      However, when using the 'insertElementBefore' function, it is assumed that all duplicate elements are contained in the duplicates array.

      I've attached the one-line patch.

      Note the test below uses the course edit form for convenience, as it is already set up, but this will affect any forms with multiple unnamed entries.

        Gliffy Diagrams

          Activity

          Hide
          salvetore Michael de Raadt added a comment -

          Hi, Davo.

          Thanks for reporting that and providing a solution.

          Show
          salvetore Michael de Raadt added a comment - Hi, Davo. Thanks for reporting that and providing a solution.
          Hide
          poltawski Dan Poltawski added a comment -

          Hi Davo,

          Is this still an issue? If so perhaps you could rebase and put up for peer review? Thanks

          Show
          poltawski Dan Poltawski added a comment - Hi Davo, Is this still an issue? If so perhaps you could rebase and put up for peer review? Thanks
          Hide
          davosmith Davo Smith added a comment -

          Just checked - certainly is still an issue and the 1-line fix is unchanged (although I've cherry-picked it into a new branch on top of master).

          Show
          davosmith Davo Smith added a comment - Just checked - certainly is still an issue and the 1-line fix is unchanged (although I've cherry-picked it into a new branch on top of master).
          Hide
          poltawski Dan Poltawski added a comment -

          Looks good to me.

          Show
          poltawski Dan Poltawski added a comment - Looks good to me.
          Hide
          davosmith Davo Smith added a comment -

          If all OK, could it be put forward for integration review?

          Show
          davosmith Davo Smith added a comment - If all OK, could it be put forward for integration review?
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Some hours ago...

          the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Some hours ago... the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks Davo, this has been integrated now.
          I'd only been integrated on master, 22, and 21 though sorry. 20 and 19 branches are now only receiving security patches or patches that have been agreed upon and are considered necessary.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks Davo, this has been integrated now. I'd only been integrated on master, 22, and 21 though sorry. 20 and 19 branches are now only receiving security patches or patches that have been agreed upon and are considered necessary. Cheers Sam
          Hide
          rwijaya Rossiani Wijaya added a comment -

          This is looking great.

          Test passed.

          Show
          rwijaya Rossiani Wijaya added a comment - This is looking great. Test passed.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          FCT (fixed, closing, thanks). Ciao

          "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!"
          ~ Benjamin Disraeli

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - FCT (fixed, closing, thanks). Ciao "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!" ~ Benjamin Disraeli

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

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