瀏覽代碼

Fix autocomplete issues in change password form (#7812)

## Screenshots

<details>
<summary>Before</summary>

<img width="773" height="652" alt="image" src="https://github.com/user-attachments/assets/89a0e58c-8c4a-41ff-b5d6-3e916079d563" />

</details>

<details>
<summary>After</summary>

<img width="1006" height="646" alt="image" src="https://github.com/user-attachments/assets/f4575103-7365-4870-a170-2742bf10eb27" />

</details>

This is an example on Firefox, where the `Master authentication token` field was incorrectly being autofilled.
Red borders are indicating that the fields are required.

## List of changes

* `required="required"` is now being added to the password fields if the section is open
* The `challenge` field is being added if section is open instead of when at least one of the password fields isn't empty due to autocomplete
* Added `autocomplete="new-password"` on fields that shouldn't be autocompleted
   * Unfortunately Chrome requires a workaround with CSS
   * Not tested on Safari yet
* User will be redirected to profile page after successfully changing their password instead of index page

## How to test

Autocomplete related changes should be tested on a HTTPS page with saved credentials for FreshRSS
Inverle 6 月之前
父節點
當前提交
bf6e634e04

+ 1 - 3
app/Controllers/userController.php

@@ -180,10 +180,8 @@ class FreshRSS_user_Controller extends FreshRSS_ActionController {
 			if ($ok) {
 				if (FreshRSS_Context::systemConf()->force_email_validation && $email !== $old_email) {
 					Minz_Request::good(_t('feedback.profile.updated'), ['c' => 'user', 'a' => 'validateEmail']);
-				} elseif ($newPasswordPlain == '') {
-					Minz_Request::good(_t('feedback.profile.updated'), ['c' => 'user', 'a' => 'profile']);
 				} else {
-					Minz_Request::good(_t('feedback.profile.updated'), ['c' => 'index', 'a' => 'index']);
+					Minz_Request::good(_t('feedback.profile.updated'), ['c' => 'user', 'a' => 'profile']);
 				}
 			} else {
 				Minz_Request::bad(_t('feedback.profile.error'), ['c' => 'user', 'a' => 'profile']);

+ 8 - 6
app/views/user/profile.phtml

@@ -32,7 +32,9 @@
 		<div class="form-group">
 			<label class="group-name" for="email"><?= _t('conf.profile.email') ?></label>
 			<div class="group-controls">
-				<input id="email" name="email" type="email" autocomplete="new-password" value="<?= FreshRSS_Context::userConf()->mail_login ?>" />
+				<!-- Workaround for Chrome, related to change password section -->
+				<input id="ignore" class="ignore-auto-complete" type="text" tabindex="-1" aria-hidden="true" data-no-leave-validation="1" />
+				<input id="email" name="email" type="email" value="<?= FreshRSS_Context::userConf()->mail_login ?>" autocomplete="new-password" />
 			</div>
 		</div>
 
@@ -41,7 +43,7 @@
 			<label class="group-name" for="token"><?= _t('admin.auth.token') ?></label>
 			<?php $token = FreshRSS_Context::userConf()->token; ?>
 			<div class="group-controls">
-				<input type="text" id="token" name="token" value="<?= $token ?>" placeholder="<?= _t('gen.short.blank_to_disable') ?>" />
+				<input id="token" name="token" type="text" value="<?= $token ?>" placeholder="<?= _t('gen.short.blank_to_disable') ?>" autocomplete="new-password" />
 				<p class="help"><?= _i('help') ?> <?= _t('admin.auth.token_help') ?></p>
 				<kbd><?= Minz_Url::display(['a' => 'rss', 'params' => ['user' => Minz_User::name() ?? '',
 					'token' => $token, 'hours' => FreshRSS_Context::userConf()->since_hours_posts_per_rss]], 'html', true) ?></kbd>
@@ -55,14 +57,14 @@
 			Minz_Session::_param('open', false);
 		?>
 
-		<details class="form-advanced" data-challenge-if-not-empty="1"<?= $open ? ' open="open"' : ''?>>
+		<details class="form-advanced" data-challenge-if-open="1"<?= $open ? ' open="open"' : ''?>>
 			<summary class="form-advanced-title"><?= _t('conf.profile.change_password') ?></summary>
 			<div class="form-group">
 				<label class="group-name" for="currentPasswordPlain"><?= _t('conf.profile.current_password') ?></label>
 				<div class="group-controls">
 					<input type="hidden" id="username" value="<?= Minz_User::name() ?? '' ?>" />
 					<div class="stick">
-						<input type="password" id="currentPasswordPlain" class="passwordPlain" />
+						<input type="password" id="currentPasswordPlain" class="passwordPlain" data-required-if-open="1" data-no-leave-validation="1" />
 						<button type="button" class="btn toggle-password"><?= _i('key') ?></button>
 					</div>
 
@@ -76,7 +78,7 @@
 				<label class="group-name" for="newPasswordPlain"><?= _t('conf.profile.new_password') ?></label>
 				<div class="group-controls">
 					<div class="stick">
-						<input type="password" id="newPasswordPlain" name="newPasswordPlain" autocomplete="new-password" pattern=".{7,}" />
+						<input type="password" id="newPasswordPlain" name="newPasswordPlain" autocomplete="new-password" pattern=".{7,}" data-required-if-open="1" />
 						<button type="button" class="btn toggle-password"><?= _i('key') ?></button>
 					</div>
 					<p class="help">
@@ -88,7 +90,7 @@
 				<label class="group-name" for="confirmPasswordPlain"><?= _t('conf.profile.confirm_new_password') ?></label>
 				<div class="group-controls">
 					<div class="stick">
-						<input type="password" id="confirmPasswordPlain" name="confirmPasswordPlain" autocomplete="new-password" pattern=".{7,}" />
+						<input type="password" id="confirmPasswordPlain" name="confirmPasswordPlain" autocomplete="new-password" pattern=".{7,}" data-required-if-open="1" />
 						<button type="button" class="btn toggle-password"><?= _i('key') ?></button>
 					</div>
 				</div>

+ 22 - 2
p/scripts/extra.js

@@ -36,8 +36,8 @@ function init_crypto_forms() {
 		crypto_form.onsubmit = function (e) {
 			let challenge = crypto_form.querySelector('#challenge');
 			if (!challenge) {
-				crypto_form.querySelectorAll('[data-challenge-if-not-empty] input[type="password"]').forEach(el => {
-					if (el.value !== '' && !challenge) {
+				crypto_form.querySelectorAll('details[data-challenge-if-open]').forEach(el => {
+					if (el.open && !challenge) {
 						crypto_form.insertAdjacentHTML('beforeend', '<input type="hidden" id="challenge" name="challenge" />');
 						challenge = crypto_form.querySelector('#challenge');
 					}
@@ -485,6 +485,25 @@ function init_configuration_alert() {
 	};
 }
 
+function init_details_attributes() {
+	function toggleRequired(details) {
+		details.querySelectorAll('[data-required-if-open]').forEach(el => {
+			if (details.open) {
+				el.setAttribute('required', 'required');
+			} else {
+				el.removeAttribute('required');
+			}
+		});
+	}
+
+	document.querySelectorAll('details').forEach(details => {
+		details.addEventListener('toggle', () => {
+			toggleRequired(details);
+		});
+		toggleRequired(details);
+	});
+}
+
 function init_extra_afterDOM() {
 	if (!window.context) {
 		if (window.console) {
@@ -504,6 +523,7 @@ function init_extra_afterDOM() {
 		init_configuration_alert();
 		init_2stateButton();
 		init_update_feed();
+		init_details_attributes();
 
 		data_auto_leave_validation(document.body);
 

+ 7 - 0
p/themes/base-theme/frss.css

@@ -530,6 +530,13 @@ input#favicon-upload {
 	display: inline;
 }
 
+input.ignore-auto-complete {
+	position: absolute;
+	left: -9999px;
+	pointer-events: none;
+	user-select: none;
+}
+
 /*=== Buttons */
 button[disabled] {
 	opacity: 0.5;

+ 7 - 0
p/themes/base-theme/frss.rtl.css

@@ -530,6 +530,13 @@ input#favicon-upload {
 	display: inline;
 }
 
+input.ignore-auto-complete {
+	position: absolute;
+	right: -9999px;
+	pointer-events: none;
+	user-select: none;
+}
+
 /*=== Buttons */
 button[disabled] {
 	opacity: 0.5;