Skip to content

Comments

[4.0] Displaying password minimum requirements in frontend registration#30456

Closed
infograf768 wants to merge 1 commit intojoomla:4.0-devfrom
infograf768:4.0_registration_rules
Closed

[4.0] Displaying password minimum requirements in frontend registration#30456
infograf768 wants to merge 1 commit intojoomla:4.0-devfrom
infograf768:4.0_registration_rules

Conversation

@infograf768
Copy link
Member

@infograf768 infograf768 commented Aug 23, 2020

Following #30437

Summary of Changes

Added a tooltip displaying the password minimum Requirements when registering in frontend
The new xml element is rules="true"

As I was at it I modified the tooltip display in Cassiopea as the default bootstrap white letters on black background was really ugly
This aspect can be modified by the people working on Cassiopea (if they prefer something else.)

Testing Instructions

Patch. Run npm ci
Define Password requirements in Users Options=>Tab Password Options

Actual result BEFORE applying this Pull Request

The password field does not indicate any requirements, forcing the new user to enter stuff and then modify depending on the Errors messages.

Expected result AFTER applying this Pull Request

Screen Shot 2020-08-23 at 17 49 36

or if different requirements

Screen Shot 2020-08-23 at 18 18 50

Note

If this accepted, we can implement in other core places.

Documentation Changes Required

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Aug 23, 2020
@Quy
Copy link
Contributor

Quy commented Aug 23, 2020

Add a description attribute to the xml file to produce duplicate id and aria-describedby.

30456

@infograf768
Copy link
Member Author

@Quy
As far as I know, the code produced here is a11y ok. I am not sure we need a description.
See code results for the admin Search field in articles manager.
Will look again tomorrow anyway.

@brianteeman
Copy link
Contributor

The code is correct. Its the only field on the user registration page that doesn't have accessibility errors.

The strings should match the strings in the admin
image

image

@infograf768
Copy link
Member Author

@Quy
I guess I now understand what you meant,
By design, the intention is to never use both a description and rules for the password field.
We use the description in Installation and should use the tooltip in other cases.

@infograf768
Copy link
Member Author

infograf768 commented Aug 24, 2020

@brianteeman
My intention was to modify the backend labels.
Integers may not be and obvious word for many except developers/math people (anyway nobody is going to use figures like 1.5...) but Digits is fine in every language.
Length is imprecise because it is a neutral term. We are speaking here of a number of Characters. So, let's call it that way.

Also, if you look at the strings errors, they do use Digits and Characters
JFIELD_PASSWORD_NOT_ENOUGH_INTEGERS_N="Password does not have enough digits. At least %s digits are required."
JFIELD_PASSWORD_TOO_SHORT_N="Password is too short. Passwords must have at least %s characters."
etc.

@Magnytu2
Copy link

I have in information only the number of characters. Is it right ?
Capture d’écran 2020-08-24 à 18 30 35

@richard67
Copy link
Member

@Magnytu2 Yes, because you have not changed password rules so they contain more requirements than the length. You can do that in com_users options on tab "Password Options". Please do so and check the result. Thanks in advance.

@Magnytu2
Copy link

Yes everything works, except with the @ which must not be a symbol ? But I am happy to discover this function which I did not know.

@infograf768
Copy link
Member Author

Please mark your test OK on issues.joomla.org

@Magnytu2
Copy link

I have tested this item ✅ successfully on 03d1366

Test ok


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30456.

@Quy
Copy link
Contributor

Quy commented Aug 24, 2020

Would this be better without having to add the rule attribute and tooltip and fix the duplication?

password

if (!empty($description))
{
	$requirements = [];

	if ($minLength)
	{
		$requirements[] = Text::sprintf('JFIELD_PASSWORD_RULES_CHARACTERS', $minLength);
	}

	if ($minIntegers)
	{
		$requirements[] = Text::sprintf('JFIELD_PASSWORD_RULES_DIGITS', $minIntegers);
	}

	if ($minSymbols)
	{
		$requirements[] = Text::sprintf('JFIELD_PASSWORD_RULES_SYMBOLS', $minSymbols);
	}

	if ($minUppercase)
	{
		$requirements[] = Text::sprintf('JFIELD_PASSWORD_RULES_UPPERCASE', $minUppercase);
	}

	if ($minLowercase)
	{
		$requirements[] = Text::sprintf('JFIELD_PASSWORD_RULES_LOWERCASE', $minLowercase);
	}
}
?>
<?php if (!empty($description)) : ?>
	<div id="<?php echo $name . '-desc'; ?>" class="small text-muted">
		<?php echo Text::sprintf($description, implode(', ', $requirements)); ?>
	</div>
<?php endif; ?>

Add to registration.xml:
description="JFIELD_PASSWORD_RULES_MINIMUM_REQUIREMENTS"

Change:
JFIELD_PASSWORD_RULES_CHARACTERS="Characters: %d"

Add:
JFIELD_PASSWORD_RULES_MINIMUM_REQUIREMENTS="<strong>Minimum Requirements</strong> %s"

@brianteeman
Copy link
Contributor

@infograf768 Great that you want to change both sets of strings to match

I would prefer the much simpler "numbers" to either "digits" or "integers"

@infograf768
Copy link
Member Author

@zero-24
I had figured you would have preferred such a display.
I already had done here a variation, keeping the rules attribute.
Will try your proposal. Evidently, it would imply modifying the J installation password tip as done in #30402 as I am not sure it will get the minimum characters

Will come back after tests.

@zero-24
Copy link
Contributor

zero-24 commented Aug 25, 2020

I'm not sure whether that was for me but to answer your question when you can not get a value from the Database just fallback to the default values ;-)

My proposal would be to show all of that next the message that the PW does not meet the requirements. As all that info should be in JS anyway but that is beyond my knowledge of JS at that point :-D

@zero-24
Copy link
Contributor

zero-24 commented Aug 25, 2020

But by Default it is only the 12 chars so the installer should be fine as it is and should only get the 'do not show the requiremets' xml switch so we do not dublicate that info.

@infograf768
Copy link
Member Author

I would prefer the much simpler "numbers" to either "digits" or "integers"

Not sure, digit is a single number. Number may be composed of one or multiple digits.

@infograf768
Copy link
Member Author

infograf768 commented Aug 25, 2020

@zero-24

But by Default it is only the 12 chars so the installer should be fine as it is

I guess so but I also have to modify its description string, i.e. use the value of
description="JFIELD_PASSWORD_RULES_MINIMUM_REQUIREMENTS"
instead of the one of
description="INSTL_ADMIN_PASSWORD_LENGTH"

As I said, will first make a test and close this PR and create a new one.

@brianteeman
Copy link
Contributor

I would prefer the much simpler "numbers" to either "digits" or "integers"

Not sure, digit is a single number. Number may be composed of one or multiple digits.

In English it is perfectly normal and expected to refer to numbers and letters in this use case

@infograf768
Copy link
Member Author

Oxford Dictionnary
https://www.lexico.com/definition/digit

Any of the numerals from 0 to 9, especially when forming part of a number.

I will stick to it.

@brianteeman
Copy link
Contributor

A dictionary and real world usage are very different.

But as you always think you know better about a language that is not your native language just see what everyone else does. 

https://www.google.com/search?q=password+requirements&tbm=isch&safe=strict&chips=q:password+requirements,g_1:strong:xOzZWoEsaS8%3D&safe=strict&hl=en&sa=X&ved=2ahUKEwi13JD_2bXrAhVE_hQKHRWRAAIQ4lYoCXoECAEQJw&biw=1867&bih=891#imgrc=At05L06IxSmxUM  

Feel free to write what you want in the french translation but in this use case the correct english is "numbers"

@infograf768
Copy link
Member Author

In your list there are almost as many numbers of examples using number than digit
It's not a good way to improve bad habits.
You will be free to change all if you desire in your own PR.

https://community.ui.com/questions/airControl-2-Password-Requirements/068eeb2e-6c6d-49b6-b7a2-81f9d61a9a34
https://en.wikipedia.org/wiki/Password_strength#/media/File:KeePass_random_password.png
https://i.dailymail.co.uk/i/pix/2017/05/08/23/401C689F00000578-0-image-m-70_1494282479950.jpg

@brianteeman
Copy link
Contributor

So you found three bad examples to prove your incorrect point.

If google, facebook and twitter all use numbers thats good enough proof for me

@infograf768
Copy link
Member Author

Closed in favor of #30473 to implement description tip instead of tooltip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants