From 1e1ab336259bbf336ab4eb08ad8a9c3bfb41aae1 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Mon, 14 Jan 2019 16:14:45 +0800 Subject: [PATCH] MDL-64570 accessibility: autocomplete user profile fields Only if editing your own user details and not logged in as someone else. --- lib/behat/form_field/behat_form_field.php | 10 +++++ login/change_password_form.php | 6 ++- login/forgot_password_form.php | 9 +++- .../core_form/element-password.mustache | 2 +- user/editadvanced_form.php | 8 +++- user/editlib.php | 21 ++++++--- user/language_form.php | 5 ++- user/lib.php | 39 +++++++++++++++++ user/tests/behat/behat_user.php | 32 ++++++++++++++ user/tests/behat/input-purpose.feature | 43 +++++++++++++++++++ 10 files changed, 161 insertions(+), 14 deletions(-) create mode 100644 user/tests/behat/input-purpose.feature diff --git a/lib/behat/form_field/behat_form_field.php b/lib/behat/form_field/behat_form_field.php index d6da75c63c4..bb011e0a256 100644 --- a/lib/behat/form_field/behat_form_field.php +++ b/lib/behat/form_field/behat_form_field.php @@ -136,6 +136,16 @@ class behat_form_field { return $instance->matches($expectedvalue); } + /** + * Get the value of an attribute set on this field. + * + * @param string $name The attribute name + * @return string The attribute value + */ + public function get_attribute($name) { + return $this->field->getAttribute($name); + } + /** * Guesses the element type we are dealing with in case is not a text-based element. * diff --git a/login/change_password_form.php b/login/change_password_form.php index 50646907b89..4a2597932c1 100644 --- a/login/change_password_form.php +++ b/login/change_password_form.php @@ -26,7 +26,8 @@ defined('MOODLE_INTERNAL') || die(); -require_once $CFG->libdir.'/formslib.php'; +require_once($CFG->libdir.'/formslib.php'); +require_once($CFG->dirroot.'/user/lib.php'); class login_change_password_form extends moodleform { @@ -51,7 +52,8 @@ class login_change_password_form extends moodleform { if ($policies) { $mform->addElement('static', 'passwordpolicyinfo', '', implode('
', $policies)); } - $mform->addElement('password', 'password', get_string('oldpassword')); + $purpose = user_edit_map_field_purpose($USER->id, 'password'); + $mform->addElement('password', 'password', get_string('oldpassword'), $purpose); $mform->addRule('password', get_string('required'), 'required', null, 'client'); $mform->setType('password', PARAM_RAW); diff --git a/login/forgot_password_form.php b/login/forgot_password_form.php index 32e5310fbfc..67a4d9f84cf 100644 --- a/login/forgot_password_form.php +++ b/login/forgot_password_form.php @@ -25,6 +25,7 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->libdir.'/formslib.php'); +require_once($CFG->dirroot.'/user/lib.php'); /** * Reset forgotten password form definition. @@ -40,12 +41,15 @@ class login_forgot_password_form extends moodleform { * Define the forgot password form. */ function definition() { + global $USER; + $mform = $this->_form; $mform->setDisableShortforms(true); $mform->addElement('header', 'searchbyusername', get_string('searchbyusername'), ''); - $mform->addElement('text', 'username', get_string('username')); + $purpose = user_edit_map_field_purpose($USER->id, 'username'); + $mform->addElement('text', 'username', get_string('username'), 'size="20"' . $purpose); $mform->setType('username', PARAM_RAW); $submitlabel = get_string('search'); @@ -53,7 +57,8 @@ class login_forgot_password_form extends moodleform { $mform->addElement('header', 'searchbyemail', get_string('searchbyemail'), ''); - $mform->addElement('text', 'email', get_string('email')); + $purpose = user_edit_map_field_purpose($USER->id, 'email'); + $mform->addElement('text', 'email', get_string('email'), 'maxlength="100" size="30"' . $purpose); $mform->setType('email', PARAM_RAW_TRIMMED); $submitlabel = get_string('search'); diff --git a/theme/boost/templates/core_form/element-password.mustache b/theme/boost/templates/core_form/element-password.mustache index b2532fb62e3..09f779f2bb7 100644 --- a/theme/boost/templates/core_form/element-password.mustache +++ b/theme/boost/templates/core_form/element-password.mustache @@ -9,7 +9,7 @@ size="{{element.size}}" {{#error}} autofocus aria-describedby="id_error_{{element.name}}" - {{/error}} {{{attributes}}}> + {{/error}} {{{element.attributes}}}> {{/element.frozen}} {{/element}} {{/ core_form/element-template }} diff --git a/user/editadvanced_form.php b/user/editadvanced_form.php index fe2fd312bce..31dd330cd85 100644 --- a/user/editadvanced_form.php +++ b/user/editadvanced_form.php @@ -27,6 +27,7 @@ if (!defined('MOODLE_INTERNAL')) { } require_once($CFG->dirroot.'/lib/formslib.php'); +require_once($CFG->dirroot.'/user/lib.php'); /** * Class user_editadvanced_form. @@ -96,7 +97,8 @@ class user_editadvanced_form extends moodleform { } } - $mform->addElement('text', 'username', get_string('username'), 'size="20"'); + $purpose = user_edit_map_field_purpose($userid, 'username'); + $mform->addElement('text', 'username', get_string('username'), 'size="20"' . $purpose); $mform->addHelpButton('username', 'username', 'auth'); $mform->setType('username', PARAM_RAW); @@ -116,7 +118,9 @@ class user_editadvanced_form extends moodleform { if (!empty($CFG->passwordpolicy)) { $mform->addElement('static', 'passwordpolicyinfo', '', print_password_policy()); } - $mform->addElement('passwordunmask', 'newpassword', get_string('newpassword'), 'size="20"'); + + $purpose = user_edit_map_field_purpose($userid, 'password'); + $mform->addElement('passwordunmask', 'newpassword', get_string('newpassword'), 'size="20"' . $purpose); $mform->addHelpButton('newpassword', 'newpassword'); $mform->setType('newpassword', core_user::get_property_type('password')); $mform->disabledIf('newpassword', 'createpassword', 'checked'); diff --git a/user/editlib.php b/user/editlib.php index d5bbd1ed4f7..270621415cc 100644 --- a/user/editlib.php +++ b/user/editlib.php @@ -22,6 +22,8 @@ * @package core_user */ +require_once($CFG->dirroot . '/user/lib.php'); + /** * Cancels the requirement for a user to update their email address. * @@ -267,7 +269,8 @@ function useredit_shared_definition(&$mform, $editoroptions, $filemanageroptions // Add the necessary names. foreach (useredit_get_required_name_fields() as $fullname) { - $mform->addElement('text', $fullname, get_string($fullname), 'maxlength="100" size="30"'); + $purpose = user_edit_map_field_purpose($user->id, $fullname); + $mform->addElement('text', $fullname, get_string($fullname), 'maxlength="100" size="30"' . $purpose); if ($stringman->string_exists('missing'.$fullname, 'core')) { $strmissingfield = get_string('missing'.$fullname, 'core'); } else { @@ -280,7 +283,8 @@ function useredit_shared_definition(&$mform, $editoroptions, $filemanageroptions $enabledusernamefields = useredit_get_enabled_name_fields(); // Add the enabled additional name fields. foreach ($enabledusernamefields as $addname) { - $mform->addElement('text', $addname, get_string($addname), 'maxlength="100" size="30"'); + $purpose = user_edit_map_field_purpose($user->id, $addname); + $mform->addElement('text', $addname, get_string($addname), 'maxlength="100" size="30"' . $purpose); $mform->setType($addname, PARAM_NOTAGS); } @@ -291,7 +295,8 @@ function useredit_shared_definition(&$mform, $editoroptions, $filemanageroptions . get_string('emailchangecancel', 'auth') . ''; $mform->addElement('static', 'emailpending', get_string('email'), $notice); } else { - $mform->addElement('text', 'email', get_string('email'), 'maxlength="100" size="30"'); + $purpose = user_edit_map_field_purpose($user->id, 'email'); + $mform->addElement('text', 'email', get_string('email'), 'maxlength="100" size="30"' . $purpose); $mform->addRule('email', $strrequired, 'required', null, 'client'); $mform->setType('email', PARAM_RAW_TRIMMED); } @@ -310,9 +315,10 @@ function useredit_shared_definition(&$mform, $editoroptions, $filemanageroptions $mform->setDefault('city', $CFG->defaultcity); } + $purpose = user_edit_map_field_purpose($user->id, 'country'); $choices = get_string_manager()->get_list_of_countries(); $choices = array('' => get_string('selectacountry') . '...') + $choices; - $mform->addElement('select', 'country', get_string('selectacountry'), $choices); + $mform->addElement('select', 'country', get_string('selectacountry'), $choices, $purpose); if (!empty($CFG->country)) { $mform->setDefault('country', core_user::get_property_default('country')); } @@ -328,7 +334,9 @@ function useredit_shared_definition(&$mform, $editoroptions, $filemanageroptions } if ($user->id < 0) { - $mform->addElement('select', 'lang', get_string('preferredlanguage'), get_string_manager()->get_list_of_translations()); + $purpose = user_edit_map_field_purpose($user->id, 'lang'); + $translations = get_string_manager()->get_list_of_translations(); + $mform->addElement('select', 'lang', get_string('preferredlanguage'), $translations, $purpose); $lang = empty($user->lang) ? $CFG->lang : $user->lang; $mform->setDefault('lang', $lang); } @@ -375,7 +383,8 @@ function useredit_shared_definition(&$mform, $editoroptions, $filemanageroptions if (count($disabledusernamefields) > 0) { $mform->addElement('header', 'moodle_additional_names', get_string('additionalnames')); foreach ($disabledusernamefields as $allname) { - $mform->addElement('text', $allname, get_string($allname), 'maxlength="100" size="30"'); + $purpose = user_edit_map_field_purpose($user->id, $allname); + $mform->addElement('text', $allname, get_string($allname), 'maxlength="100" size="30"' . $purpose); $mform->setType($allname, PARAM_NOTAGS); } } diff --git a/user/language_form.php b/user/language_form.php index 4dab6bed2fa..50d091130e2 100644 --- a/user/language_form.php +++ b/user/language_form.php @@ -27,6 +27,7 @@ if (!defined('MOODLE_INTERNAL')) { } require_once($CFG->dirroot.'/lib/formslib.php'); +require_once($CFG->dirroot.'/user/lib.php'); /** * Class user_edit_form. @@ -57,7 +58,9 @@ class user_edit_language_form extends moodleform { $mform->addElement('hidden', 'course', $COURSE->id); $mform->setType('course', PARAM_INT); - $mform->addElement('select', 'lang', get_string('preferredlanguage'), get_string_manager()->get_list_of_translations()); + $purpose = user_edit_map_field_purpose($userid, 'lang'); + $translations = get_string_manager()->get_list_of_translations(); + $mform->addElement('select', 'lang', get_string('preferredlanguage'), $translations, $purpose); $mform->setDefault('lang', core_user::get_property_default('lang')); $this->add_action_buttons(true, get_string('savechanges')); diff --git a/user/lib.php b/user/lib.php index ea1c1cb829a..e044f02da93 100644 --- a/user/lib.php +++ b/user/lib.php @@ -1515,3 +1515,42 @@ function core_user_inplace_editable($itemtype, $itemid, $newvalue) { return \core_user\output\user_roles_editable::update($itemid, $newvalue); } } + +/** + * Map an internal field name to a valid purpose from: "https://www.w3.org/TR/WCAG21/#input-purposes" + * + * @param integer $userid + * @param string $fieldname + * @return string $purpose (empty string if there is no mapping). + */ +function user_edit_map_field_purpose($userid, $fieldname) { + global $USER; + + $currentuser = ($userid == $USER->id) && !\core\session\manager::is_loggedinas(); + // These are the fields considered valid to map and auto fill from a browser. + // We do not include fields that are in a collapsed section by default because + // the browser could auto-fill the field and cause a new value to be saved when + // that field was never visible. + $validmappings = array( + 'username' => 'username', + 'password' => 'current-password', + 'firstname' => 'given-name', + 'lastname' => 'family-name', + 'middlename' => 'additional-name', + 'email' => 'email', + 'country' => 'country', + 'lang' => 'language' + ); + + $purpose = ''; + if (!$currentuser) { + // Do not set a purpose. + $purpose = ''; + } + if (isset($validmappings[$fieldname])) { + $purpose = ' autocomplete="' . $validmappings[$fieldname] . '" '; + } + + return $purpose; +} + diff --git a/user/tests/behat/behat_user.php b/user/tests/behat/behat_user.php index 36339e64276..2a6f3dac7e1 100644 --- a/user/tests/behat/behat_user.php +++ b/user/tests/behat/behat_user.php @@ -53,4 +53,36 @@ class behat_user extends behat_base { $this->execute("behat_general::i_click_on", array("//select[@id='formactionid']" . "/option[contains(., " . $nodetext . ")]", "xpath_element")); } + + /** + * The input field should have autocomplete set to this value. + * + * @Then /^the field "(?P(?:[^"]|\\")*)" should have purpose "(?P(?:[^"]|\\")*)"$/ + * @param string $field The field to select. + * @param string $purpose The expected purpose. + */ + public function the_field_should_have_purpose($field, $purpose) { + $fld = behat_field_manager::get_form_field_from_label($field, $this); + + $value = $fld->get_attribute('autocomplete'); + if ($value != $purpose) { + throw new ExpectationException('The "' . $field . '" field does not have purposea "' . $purpose . '"', $this->getSession()); + } + } + + /** + * The input field should not have autocomplete set to this value. + * + * @Then /^the field "(?P(?:[^"]|\\")*)" should not have purpose "(?P(?:[^"]|\\")*)"$/ + * @param string $field The field to select. + * @param string $purpose The expected purpose we do not want. + */ + public function the_field_should_not_have_purpose($field, $purpose) { + $fld = behat_field_manager::get_form_field_from_label($field, $this); + + $value = $fld->get_attribute('autocomplete'); + if ($value == $purpose) { + throw new ExpectationException('The "' . $field . '" field does have purposea "' . $purpose . '"', $this->getSession()); + } + } } diff --git a/user/tests/behat/input-purpose.feature b/user/tests/behat/input-purpose.feature new file mode 100644 index 00000000000..f466f65e5a0 --- /dev/null +++ b/user/tests/behat/input-purpose.feature @@ -0,0 +1,43 @@ +@core @core_user +Feature: The purpose of each input field collecting information about the user can be determined + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | unicorn | unicorn | 1 | unicorn@example.com | + And the following "courses" exist: + | fullname | shortname | category | groupmode | + | Course 1 | C1 | 0 | 1 | + And the following "course enrolments" exist: + | user | course | role | + | unicorn | C1 | student | + + @javascript + Scenario: Fields for other users are not auto filled + When I log in as "admin" + And I navigate to "Users > Accounts > Browse list of users" in site administration + And I click on ".icon[title=Edit]" "css_element" in the "unicorn@example.com" "table_row" + And I expand all fieldsets + Then the field "Username" should not have purpose "username" + And the field "First name" should not have purpose "given-name" + And the field "Surname" should not have purpose "family-name" + And the field "Email" should not have purpose "email" + And the field "Select a country" should not have purpose "country" + And I press "Cancel" + And I follow "Preferred language" + And the field "Preferred language" should not have purpose "language" + + @javascript + Scenario: My own user fields are auto filled + When I log in as "unicorn" + And I follow "Profile" in the user menu + And I click on "Edit profile" "link" in the "region-main" "region" + And I expand all fieldsets + Then the field "First name" should have purpose "given-name" + And the field "Surname" should have purpose "family-name" + And the field "Email" should have purpose "email" + And the field "Select a country" should have purpose "country" + And I press "Cancel" + And I follow "Preferences" in the user menu + And I follow "Preferred language" + And the field "Preferred language" should have purpose "language"