From b7205c5c8432ef883bbad6d2b1b2800726c0ffdc Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Mon, 20 Nov 2023 18:07:35 +0000 Subject: [PATCH] MDL-71336 forms: ensure grouped date elements have unique IDs. Previously they were each taking the ID of their parent group element. --- lib/form/dateselector.php | 5 +++-- lib/form/datetimeselector.php | 13 +++++++------ lib/form/duration.php | 7 ++----- lib/form/group.php | 10 ++++++++++ lib/form/upgrade.txt | 2 ++ 5 files changed, 24 insertions(+), 13 deletions(-) diff --git a/lib/form/dateselector.php b/lib/form/dateselector.php index dbe81b09fe3..0f9ec2cbfcd 100644 --- a/lib/form/dateselector.php +++ b/lib/form/dateselector.php @@ -130,11 +130,12 @@ class MoodleQuickForm_date_selector extends MoodleQuickForm_group { // If optional we add a checkbox which the user can use to turn if on. if ($this->_options['optional']) { $this->_elements[] = $this->createFormElement('checkbox', 'enabled', null, - get_string('enable'), $this->getAttributes(), true); + get_string('enable'), $this->getAttributesForFormElement(), true); } foreach ($dateformat as $key => $value) { // E_STRICT creating elements without forms is nasty because it internally uses $this - $this->_elements[] = $this->createFormElement('select', $key, get_string($key, 'form'), $value, $this->getAttributes(), true); + $this->_elements[] = $this->createFormElement('select', $key, get_string($key, 'form'), $value, + $this->getAttributesForFormElement(), true); } // The YUI2 calendar only supports the gregorian calendar type so only display the calendar image if this is being used. if ($calendartype->get_name() === 'gregorian') { diff --git a/lib/form/datetimeselector.php b/lib/form/datetimeselector.php index 26b181a5e03..10446e20cac 100644 --- a/lib/form/datetimeselector.php +++ b/lib/form/datetimeselector.php @@ -134,26 +134,27 @@ class MoodleQuickForm_date_time_selector extends MoodleQuickForm_group { // If optional we add a checkbox which the user can use to turn if on. if ($this->_options['optional']) { $this->_elements[] = $this->createFormElement('checkbox', 'enabled', null, - get_string('enable'), $this->getAttributes(), true); + get_string('enable'), $this->getAttributesForFormElement(), true); } $dateformat = $calendartype->get_date_order($this->_options['startyear'], $this->_options['stopyear']); if (right_to_left()) { // Display time to the right of date, in RTL mode. $this->_elements[] = $this->createFormElement('select', 'minute', get_string('minute', 'form'), - $minutes, $this->getAttributes(), true); + $minutes, $this->getAttributesForFormElement(), true); $this->_elements[] = $this->createFormElement('select', 'hour', get_string('hour', 'form'), - $hours, $this->getAttributes(), true); + $hours, $this->getAttributesForFormElement(), true); // Reverse date element (Should be: Day, Month, Year), in RTL mode. $dateformat = array_reverse($dateformat); } foreach ($dateformat as $key => $date) { // E_STRICT creating elements without forms is nasty because it internally uses $this - $this->_elements[] = $this->createFormElement('select', $key, get_string($key, 'form'), $date, $this->getAttributes(), true); + $this->_elements[] = $this->createFormElement('select', $key, get_string($key, 'form'), $date, + $this->getAttributesForFormElement(), true); } if (!right_to_left()) { // Display time to the left of date, in LTR mode. $this->_elements[] = $this->createFormElement('select', 'hour', get_string('hour', 'form'), $hours, - $this->getAttributes(), true); + $this->getAttributesForFormElement(), true); $this->_elements[] = $this->createFormElement('select', 'minute', get_string('minute', 'form'), $minutes, - $this->getAttributes(), true); + $this->getAttributesForFormElement(), true); } // The YUI2 calendar only supports the gregorian calendar type so only display the calendar image if this is being used. if ($calendartype->get_name() === 'gregorian') { diff --git a/lib/form/duration.php b/lib/form/duration.php index aeb152a1c0f..0ff1aaf2ee2 100644 --- a/lib/form/duration.php +++ b/lib/form/duration.php @@ -172,10 +172,7 @@ class MoodleQuickForm_duration extends MoodleQuickForm_group { * Override of standard quickforms method to create this element. */ function _createElements() { - $attributes = $this->getAttributes(); - if (is_null($attributes)) { - $attributes = []; - } + $attributes = $this->getAttributesForFormElement(); if (!isset($attributes['size'])) { $attributes['size'] = 3; } @@ -191,7 +188,7 @@ class MoodleQuickForm_duration extends MoodleQuickForm_group { // If optional we add a checkbox which the user can use to turn if on if($this->_options['optional']) { $this->_elements[] = $this->createFormElement('checkbox', 'enabled', null, - get_string('enable'), $this->getAttributes(), true); + get_string('enable'), $attributes, true); } foreach ($this->_elements as $element){ if (method_exists($element, 'setHiddenLabel')){ diff --git a/lib/form/group.php b/lib/form/group.php index 53261927b9e..533c7f857bf 100644 --- a/lib/form/group.php +++ b/lib/form/group.php @@ -164,6 +164,16 @@ class MoodleQuickForm_group extends HTML_QuickForm_group implements templatable return call_user_func_array([$this->_mform, 'createElement'], func_get_args()); } + /** + * Return attributes suitable for passing to {@see createFormElement}, comprised of all group attributes without ID in + * order to ensure uniqueness of that value within the group + * + * @return array + */ + public function getAttributesForFormElement(): array { + return array_diff_key((array) $this->getAttributes(), array_flip(['id'])); + } + public function export_for_template(renderer_base $output) { global $OUTPUT; diff --git a/lib/form/upgrade.txt b/lib/form/upgrade.txt index bc43b963b54..b4a302be108 100644 --- a/lib/form/upgrade.txt +++ b/lib/form/upgrade.txt @@ -7,6 +7,8 @@ information provided here is intended especially for developers. Alongside with that new ".suggestions-heading" class was added to easily generate suggestion headings * The `core/form-autocomplete` module now exports an `enhanceField` method to return native promise (of which the previous `enhance` is now a wrapper of, while preserving jQuery promise return) +* The group element has a new method `getAttributesForFormElement` which should be used in conjunction + with `createFormElement` to ensure that all elements within the group have unique IDs === 4.3 ===