Problem/Motivation

"password matches" and "password strength" accessibility could be better. Much of the problem is that to screen readers, the content is not presented in a consistent logical fashion. Furthermore, changes to the page (password strength & password confirmation) are not passed along to the screen reader effectively via ARIA.

Proposed resolution

"password matches" indicator (label followed up by the value) should be placed after the "confirm password" field, not before.

In addition to this (I don't know if this is possible this is possible) it would be great that when a user is typing into the "confirm password" field he/she could ear when password and "confirm password" field matches, as it happens for value changes of the "password strength" indicator.

These are really important for drupal accessibility!

Remaining tasks

Find out if the improvements are possible.

yes

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Update the issue summary Instructions yes
Update the issue summary noting if allowed during the beta Instructions
Add automated tests Instructions we need more
Manually test the patch Novice Instructions
Add steps to reproduce the issue Novice Instructions
Embed before and after screenshots in the issue summary Novice Instructions
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards Instructions

User interface changes

None to sighted users.

API changes

None

Original report by @falcon03

Files: 
CommentFileSizeAuthor
#118 improve-password-matches-1811240-118.patch5.26 KBpolaki_viswanath
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,254 pass(es).
[ View ]
#111 move-password-match-and-strength-1811240-111.patch5.22 KBadci_contributor
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,910 pass(es).
[ View ]
#107 change-password-voiceover.mov3.43 MBBarisW
#103 move-password-match-and-strength-1811240-103.patch5.2 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,063 pass(es).
[ View ]
#101 Screen Shot 2014-10-05 at 9.29.26 AM.png204.25 KBmgifford
#100 move-password-match-and-strength-1811240-100.patch5.21 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,993 pass(es).
[ View ]
#99 move-password-match-and-strength-1811240-99.patch5.19 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,986 pass(es).
[ View ]
#99 Screen Shot 2014-10-05 at 8.59.32 AM.png108.53 KBmgifford
#98 move-password-match-and-strength-1811240-98.patch5.17 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,993 pass(es).
[ View ]
#90 move-password-match-and-strength-1811240-90.patch5.31 KBBarisW
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,918 pass(es).
[ View ]
#90 password-improvements.png58.81 KBBarisW
#87 move-password-match-and-strength-1811240-87.patch4.54 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,747 pass(es).
[ View ]
#86 move-password-match-and-strength-1811240-86.patch4.54 KBBarisW
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,701 pass(es).
[ View ]
#77 move-password-match-and-strength-1811240-76.patch4.56 KBmgifford
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch move-password-match-and-strength-1811240-76.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#76 Patch-Passwords-dont-match.png274.87 KBmgifford
#76 Patch-Paswords-match.png285.18 KBmgifford
#70 move-password-match-and-strength-1811240-70.patch4.54 KBlahoosascoots
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch move-password-match-and-strength-1811240-70.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#66 move-password-match-and-strength-1811240-66.patch4.54 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,480 pass(es).
[ View ]
#61 PasswordsAndScreensavers.png437.06 KBmgifford
#60 move-password-match-and-strength-1811240-59.patch4.68 KBmgifford
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch move-password-match-and-strength-1811240-59.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#58 move-password-match-and-strength-1811240-58.patch4.75 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 59,856 pass(es).
[ View ]
#57 move-password-match-and-strength-1811240-56.patch4.46 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] 59,465 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#52 move-password-match-and-strength-1811240-52.patch4.46 KBjmuzz
PASSED: [[SimpleTest]]: [MySQL] 59,831 pass(es).
[ View ]
#43 Password_secure_edit_text.png93.89 KBmgifford
#43 Confirm_password_secure_edit_text.png100.12 KBmgifford
#43 Fair.png129.65 KBmgifford
#43 Yes_not.png70.93 KBmgifford
#40 passwd.jpg56.41 KBdcrocks
#35 move-password-match-and-strength-1811240-35.patch2.96 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-35.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#32 move-password-match-and-strength-1811240-32.patch2.96 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-32.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#29 move-password-match-and-strength-1811240-29.patch2.92 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-29.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#27 move-password-match-and-strength-1811240-27.patch2.9 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 56,954 pass(es).
[ View ]
#22 move-password-match-and-strength-1811240-22b.patch2.91 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-22b.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#19 move-password-match-and-strength-1811240-19.patch6.64 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-19.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#16 move-password-match-and-strength-1811240-16.patch2.81 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 48,653 pass(es).
[ View ]
#16 PasswordStrengthAria.png194.13 KBmgifford
#10 move-password-match-and-strength-1811240-10.patch2.94 KBDevin Carlson
FAILED: [[SimpleTest]]: [MySQL] 50,757 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#8 Password_order.png189.95 KBmgifford
#7 move-password-match-and-strength-1811240-6.patch1.96 KBDevin Carlson
PASSED: [[SimpleTest]]: [MySQL] 48,220 pass(es).
[ View ]
#4 PasswordConfirmation.png155.88 KBmgifford

Comments

tim.plunkett’s picture

Category:bug» feature

Recategorizing as a feature, as indicated by the OP.

tim.plunkett’s picture

Title:Improve "password matches" accessibility» Improve "password matches" and "password strength" accessibility

Also, since #1811228: Improve password strength indicator accessibility is dealing with the same exact widget on the same form, let's merge the two.

mgifford’s picture

Just wanted to confirm that the order of the elements does not follow how they are visually presented. It would be more consistent for sure to switch the order so that the confirmation (and strength for that matter) is displayed after the password form rather than before it.

With the "confirm password" field, are you looking for a more assertive ARIA? Do you have other examples we could look at to see where this is done better?

mgifford’s picture

StatusFileSize
new155.88 KB

Attaching screenshot.

falcon03’s picture

Hi mgifford,

with "confirm password" I am referring to the field where you can enter the chosen password again (for confirmation).

Also, I would like to remember that (differently from what happens with the "password strength" indicator) the "password matches" indicator value is shown before the label (and both them are shown before the "confirm password" field).

mgifford’s picture

Issue tags:+a11ySprint

This should just be an order of operations thing. Someone needs to write the patch though.

Devin Carlson’s picture

Status:Active» Needs review
StatusFileSize
new1.96 KB
PASSED: [[SimpleTest]]: [MySQL] 48,220 pass(es).
[ View ]

A patch to change the markup order from adding the password strength indicator and passwords confirm text above the field to below the field.

Some margins had to be removed because the original placement of the text (above the fields) required some padding to move the text inline with the actual fields.

mgifford’s picture

StatusFileSize
new189.95 KB

We should have more cross browser testing, but from the looks of this patch we're very close to RTBC.

falcon03’s picture

Category:feature» task
Status:Needs review» Needs work

@Devin Carlson: ok, the "confirm password" indicator is placed after the "confirm password" field. Very good.

Since
http://drupal.org/node/1811228
has been closed as a duplicate of this issue, this patch still needs to address a problem from that issue:
the password strength indicator value should be placed after the "password strength" label, not before.

And, finally, just a wish: could we emulate the password strength indicator value behavior for the password match indicator value, so that screen readers will read the new value on change?

Btw, since this is only refactoring, I am turning this issue into a task, so that feature freeze won't block it...

Devin Carlson’s picture

Status:Needs work» Needs review
StatusFileSize
new2.94 KB
FAILED: [[SimpleTest]]: [MySQL] 50,757 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Updated patch that addresses the issues described in #9.

Changes:

  • Moved password-confirm specific variables to the top of the passwordCheckMatch function as is done with the passwordCheck function.
  • Added similar <div> and <span> wrappers to the password confirm title and text as are added to the password strength title and text.
  • Added aria-live="assertive" to the password confirm text.
  • Changed the order that password strength text vs. title is printed to make the title come before the text.
YesCT’s picture

slightly related: #432962: Add option to disable password strength checking
which found related: #1882474: Password strength indicator display broken in install process

might need a reroll (if it does and someone new wants to try, here is a reroll doc: http://drupal.org/patch/reroll)

YesCT’s picture

stevepurkiss’s picture

Interesting - I'm not sure without checking how this is affecting the work myself and @dags have been doing on #432962: Add option to disable password strength checking but FYI we've split the password strength indicator and the password matches functionality into two functions now, they were part of the same thing before. Will have a look when I get a mo (i.e. when not replying to emails I get at 3.50am ;)

Status:Needs review» Needs work

The last submitted patch, move-password-match-and-strength-1811240-10.patch, failed testing.

mgifford’s picture

Issue tags:+Needs tests

Tagging

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new194.13 KB
new2.81 KB
PASSED: [[SimpleTest]]: [MySQL] 48,653 pass(es).
[ View ]

Adding a new patch, but want to ask that the same aria tool be added to the Passwords match so that the user is alerted when no=yes -> <span class="error">no</span>

stevepurkiss’s picture

Hi again. I've just posted an updated patch for #432962-70: Add option to disable password strength checking which seems to be on the same code you're working on.

Did mention previously - not sure best approach, I guess depends on which one gets committed first? I don't think there's much of an issue, just that code is moving around so thought I'd give a heads up.

If this is not an issue for you then fine, thought better out than in ;)

mgifford’s picture

Thanks Steve. Really appreciate knowing that. I'm mostly working on just pushing ahead the patch. I'm not a JS guy. Couldn't even add the "password matches" piece with my level of JS knowledge.

Not sure that it would speed up either of the issues to try to merge them. I'm always happy for suggestions and will keep an eye on the disable password strength checking code.

mgifford’s picture

StatusFileSize
new6.64 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-19.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Since #432962-98: Add option to disable password strength checking is now RTBC, I'm just re-rolling #16 to accommodate. If that patch gets in, we should be good.

mgifford’s picture

Status:Needs review» Needs work
Issue tags:+Needs tests, +accessibility, +a11ySprint

The last submitted patch, move-password-match-and-strength-1811240-19.patch, failed testing.

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new2.91 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-22b.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Let's try that again.

mgifford’s picture

falcon03’s picture

Status:Needs review» Needs work

Testing on Windows/FF/NVDA.

  1. I "password strength" indicator is still shown before the "password" field;
  2. The "password match" indicator doesn't appear at all

So to get this issue fixed, I think we need to edit the patch to that it:

  1. Moves the password strength indicator after the "password" field;
  2. (maybe) uses the new drupalAnnounce() method for ARIA live regions;
  3. creates an ARIA live region for the "password match" indicator.
mgifford’s picture

Status:Needs work» Needs review
Issue tags:-Needs tests, -accessibility, -a11ySprint

Status:Needs review» Needs work
Issue tags:+Needs tests, +accessibility, +a11ySprint

The last submitted patch, move-password-match-and-strength-1811240-22b.patch, failed testing.

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new2.9 KB
PASSED: [[SimpleTest]]: [MySQL] 56,954 pass(es).
[ View ]

Just a re-roll.

I do think the password strength indicator is showing up after the DOM properly.. Not sure.. The aria-live is there I think.

However, haven't looked at drupalAnnounce(). But the password match indicator just doesn't seem to appear.

benjifisher’s picture

mgifford’s picture

StatusFileSize
new2.92 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-29.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Don't think this is right either, but it's now using Drupal.announce() in this patch.

mgifford’s picture

Status:Needs review» Needs work
Issue tags:+Needs tests, +accessibility, +a11ySprint

The last submitted patch, move-password-match-and-strength-1811240-29.patch, failed testing.

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new2.96 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-32.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

re-roll as user.css moved to css/user.module.css

mgifford’s picture

Status:Needs review» Needs work
Issue tags:+Needs tests, +accessibility, +a11ySprint

The last submitted patch, move-password-match-and-strength-1811240-32.patch, failed testing.

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new2.96 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch move-password-match-and-strength-1811240-35.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

re-roll

bowersox’s picture

Issue tags:+TwinCities
mgifford’s picture

geodaniel’s picture

dcrocks’s picture

StatusFileSize
new56.41 KB

Installed patch in #35 on a D8 cloned from git. The appearance is pretty bad(OS X and FF 24.0). It changes dramatically and tends to look better as the page is narrowed.
install page

dcrocks’s picture

Status:Needs review» Needs work

Patch #1839318: Replace drupal.base.css library with normalize.css for all themes went in and there appears to be some overlap with this one. Might explain my results. Needs a reroll.

dcrocks’s picture

It's not clear to me. Is this patch still needed, does the problem still exist?

mgifford’s picture

StatusFileSize
new70.93 KB
new129.65 KB
new100.12 KB
new93.89 KB

Yes @dcrocks, this is definitely still a problem.

When I tried the last patch it was definitely broken, but at the moment this functionality is broken in Core.

So firstly, with VoiceOver in Safari, in navigating down the http://sadaf3d61e4fa7e9.s3.simplytest.me/user/1#overlay=user/1/edit everything seems to work well until you hit the Password field. At that point it doesn't speak, but VoiceOver does display what it should be saying in VoiceOver. Not sure why.

Password secure edit text

The same thing happens when you go to confirm Confirmation field. It should announce that you are on that input form, but it doesn't. The text from VoiceOver looks like it does, but it was silent (unlike every other field on that page).

Confirm password secure edit text

VoiceOver does announce when there is a Fair or Good password.

Fair

But doesn't say anything if it doesn't match.

Yes not

Let me know if further confirmation is required.

mgifford’s picture

The last submitted patch, 35: move-password-match-and-strength-1811240-35.patch, failed testing.

mgifford’s picture

Issue summary:View changes
Status:Needs work» Needs review
StatusFileSize
new2.96 KB
PASSED: [[SimpleTest]]: [MySQL] 59,284 pass(es).
[ View ]

Reroll

Padma Priya Dhandapani’s picture

This patch move-password-match-and-strength-1811240-45.patch is not working.
Instead of Password strength, it is showing undefined and Passwords match is also not displaying

mgifford’s picture

mgifford’s picture

Ok, it should get set back to needs work. Also useful to produce a screenshot, but not a big deal on that.

Shyamala’s picture

Status:Needs review» Needs work

setting it to needs work

jmuzz’s picture

Status:Needs work» Needs review
StatusFileSize
new4.43 KB
FAILED: [[SimpleTest]]: [MySQL] 59,755 pass(es), 2 fail(s), and 1 exception(s).
[ View ]

I agree with replacing the aria-live tags with Drupal.announce calls because the text in the tags themselves isn't the best way to say the information being presented. The problem is that Drupal.announce doesn't output the text it's given because its purpose is to explain changes that won't necessally be represented by text on the page. Also, the announce calls should happen every time the widget changes its info, so just putting them in the page load isn't sufficient.

Here's a version of the patch that fixes the problem described in #47 as well as announcing the changes as they happen in the widget.

jmuzz’s picture

StatusFileSize
new4.46 KB
PASSED: [[SimpleTest]]: [MySQL] 59,831 pass(es).
[ View ]

Re-added 'assertive' argument to announce calls and added a missing translation call.

The last submitted patch, 51: move-password-match-and-strength-1811240-51.patch, failed testing.

mgifford’s picture

mgifford’s picture

mgifford’s picture

The confirmation boxes didn't seem to line up in my test. Also I do think the confirmation should all be in one line. Even if only for formatting purposes.

mgifford’s picture

StatusFileSize
new4.46 KB
FAILED: [[SimpleTest]]: [MySQL] 59,465 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

didn't attach it in #56 apparently.

mgifford’s picture

StatusFileSize
new4.75 KB
PASSED: [[SimpleTest]]: [MySQL] 59,856 pass(es).
[ View ]

More CSS alignment.

mgifford’s picture

Apparently that div was needed. I gotta get my local install up again.

mgifford’s picture

StatusFileSize
new4.68 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch move-password-match-and-strength-1811240-59.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
mgifford’s picture

StatusFileSize
new437.06 KB

Here's a screenshot with text for VoiceOver's feedback. It seems to work well.

The last submitted patch, 57: move-password-match-and-strength-1811240-56.patch, failed testing.

martin107’s picture

Status:Needs review» Needs work

The last submitted patch, 60: move-password-match-and-strength-1811240-59.patch, failed testing.

martin107’s picture

Issue tags:+Needs reroll
mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new4.54 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,480 pass(es).
[ View ]

Hope the bot likes this.

martin107’s picture

Issue tags:-Needs reroll
mgifford’s picture

Status:Needs review» Needs work

no longer applies.

martin107’s picture

Issue tags:+Needs reroll
lahoosascoots’s picture

Status:Needs work» Needs review
StatusFileSize
new4.54 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch move-password-match-and-strength-1811240-70.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled with no conflicts =]

lahoosascoots’s picture

Issue tags:-Needs reroll
mgifford’s picture

Status:Needs review» Needs work

This is definitely an improvement. I now get an announcement when the passwords match.

I don't get any notice for when the passwords don't match however. That might be harder to accomplish though.

It would be possible to check when the password line has the same number of characters as the confirmation line.

I don't get a notice that I have chosen a weak password and am not promted with any ideas to make it more robust.

I just did basic testing with http://www.chromevox.com/

mgifford’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 70: move-password-match-and-strength-1811240-70.patch, failed testing.

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new285.18 KB
new274.87 KB

Rerolled the code.. No new tests..

Here's with matching passwords:

And without:

mgifford’s picture

StatusFileSize
new4.56 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch move-password-match-and-strength-1811240-76.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Forgot patch...

mgifford’s picture

For TCDrupal folks wanting to test this, there's an environment up here http://s51bf82f0a91a766.s2.simplytest.me/

Login, and just start the process of changing the u/1 password. Don't hit save..

tim.plunkett’s picture

Issue tags:+JavaScript
nod_’s picture

mgifford’s picture

I do think that if the order from this issue isn't respected in #2293803 we'll get the same problem. Although I'm not certain as I haven't tested that.

Status:Needs review» Needs work

The last submitted patch, 77: move-password-match-and-strength-1811240-76.patch, failed testing.

mgifford’s picture

Issue tags:+Needs reroll

Needs a re-roll.

mgifford’s picture

Issue tags:+dcamsa11y
BarisW’s picture

Status:Needs work» Needs review
StatusFileSize
new4.54 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,701 pass(es).
[ View ]

Here's a re-roll.

mgifford’s picture

Issue tags:-Needs reroll+Amsterdam2014
StatusFileSize
new4.54 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,747 pass(es).
[ View ]

I just tested this here http://sd4ae5bf8fd014ba.s3.simplytest.me/user/1/edit

The order (placing the confirmation after the input) wasn't changed.

I'm also puzzled that I didn't hear an announcement in VoiceOver... I need to look into that more.

mgifford’s picture

Status:Needs review» Needs work

Sadly my patch messed it up more. @BarisW was better. Two things need to happen with this patch.

1) The password matches text/content should follow the input form for the confirmation password. That still isn't happening in #86 (or #87)

2) The changes of state for both the password strength and password match values should be communicated through ARIA Live.

BarisW’s picture

Assigned:Unassigned» BarisW

Hi Mike, mine was just a re-roll. I'm in the Amsterdam sprint now, will take this on.

BarisW’s picture

Assigned:BarisW» Unassigned
Status:Needs work» Needs review
StatusFileSize
new58.81 KB
new5.31 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,918 pass(es).
[ View ]

So here's a new patch. Here's the changes:

  • Both fields (password strength and password match) are now send to Drupal.announce
  • While on it, I changed two div's to span's and removed the display: inline from the css
  • I fixed the position of the confirm password
  • I set the height of the element to 0 by default to remove unneeded whitespace in the output.

I tested it with VoiceOver and that seem to work fine.

Screenshot of the changes

mgifford’s picture

Also looking at #2293803: Replace confirm password field with show/hide functionality - which completely re-does this pattern.

LewisNyman’s picture

Issue tags:+frontend, +CSS
rteijeiro’s picture

Status:Needs review» Reviewed & tested by the community

It looks sweet! Tested in Chrome, Firefox, Safari and Opera. RTBC?

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 90: move-password-match-and-strength-1811240-90.patch, failed testing.

Status:Needs work» Needs review
mgifford’s picture

Status:Needs review» Reviewed & tested by the community

setting it back to rtbc - bad bot I believe.

alexpott’s picture

Status:Reviewed & tested by the community» Needs work
+++ b/core/modules/user/css/user.module.css
@@ -41,9 +37,20 @@ input.password-field,
+  float: right; /* LTR */

Why are we aligning this to right? Looking at the screenshot above password match and password strength now are not in alignment.

mgifford’s picture

Status:Needs work» Needs review
StatusFileSize
new5.17 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,993 pass(es).
[ View ]

@alexpott - How do you keep looking at so many issues with fresh eyes? Great skill.

Anyways, The reason that the float right was there is because it was there in the design from a year ago. It matched at the time and this issues just been re-rolling changes to that old design.

But, actually, it's even bigger than this as I don't think the .password-confirm-text css should be added here. It's no longer consistent with .password-strength-text.

mgifford’s picture

StatusFileSize
new108.53 KB
new5.19 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,986 pass(es).
[ View ]

But fixing that highlights other inconsistencies between password-strength__title / password-strength__text and password-confirm-title / password-confirm-text

Which should be fixed so that:

<span class="password-strength__title">Password strength: </span>
<span class="password-strength__text">Fair</span>

Is followed rather than:

<div class="password-confirm-title">
Passwords match:
<span class="password-confirm-text ok">yes</span>
</div>
mgifford’s picture

StatusFileSize
new5.21 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,993 pass(es).
[ View ]

More changes in the JS.. There are now 2 spans. Sorry about the noise.

mgifford’s picture

StatusFileSize
new204.25 KB

Ok, attaching a screenshot. I think this patch is good now, but think there should probably be a follow-up issue. I think that "Passwords match: yes" should probably be "Passwords match: Yes" to be consistent with "Password strength: Fair" above it.

I think that's a different issue though.

hadi_gsf’s picture

I've tested the last patch, the one that Mike posted. after i'm done entering the confirmation password, i can hear "Passwords match". tested with firefox, and IE. JAWS and NVDA. I didn't hear anything about the password strength though, i had to go to the browse mode to see that. The password strength wasn't announced automatically, like the pass match did.

mgifford’s picture

StatusFileSize
new5.2 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,063 pass(es).
[ View ]

Just needed a re-roll.

mgifford’s picture

I'm a bit confused as to why this all appears at the bottom of the page:

<div id="drupal-live-announce" class="visually-hidden" aria-live="assertive" aria-busy="false">Weak password</div>
</body>

Not sure if that's why it doesn't work for @hadi_gsf

BarisW’s picture

Isn't that just what Drupal.announce does? It adds a hidden container with an aria-live="assertive" attribute.

  /**
   * Builds a div element with the aria-live attribute and attaches it
   * to the DOM.
   */
  Drupal.behaviors.drupalAnnounce = {
    attach: function (context) {
      // Create only one aria-live element.
      if (!liveElement) {
        liveElement = document.createElement('div');
        liveElement.id = 'drupal-live-announce';
        liveElement.className = 'visually-hidden';
        liveElement.setAttribute('aria-live', 'polite');
        liveElement.setAttribute('aria-busy', 'false');
        document.body.appendChild(liveElement);
      }
    }
  };
mgifford’s picture

I'm just trying to understand why there is a difference in #102.

Why would the password strength not be announced automatically, like the pass match was?

They are using the same code and show up at the bottom of the page...

@BarisW Thanks. One of these days I might have to learn JS.

BarisW’s picture

StatusFileSize
new3.43 MB

Just tested the patch in #103 and it works just as expected. I've made a screen recording to see how Voiceover behaves and it works quite good.

mgifford’s picture

I tried that http://simplytest.me/project/drupal/8.0.x

I can't get the password notification to come up on the confirmation box.

I had trouble with both while testing on both NVDA & ChromeVox.

Thanks for recording the movie.. I'm not sure what is different.

YesCT’s picture

@mgifford are you trying to reproduce a problem in head?
The link to simplytest.me that you have in #108 is not one that will have the patch applied.
http://simplytest.me/project/drupal/8.0.x?patch[]=https://www.drupal.org...

mgifford’s picture

Status:Needs review» Needs work
Issue tags:+Needs reroll
adci_contributor’s picture

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new5.22 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,910 pass(es).
[ View ]
YesCT’s picture

Issue summary:View changes

@adci_contributor Thank you for the reroll.

Are you up for adding tests? I will add instructions to the issue summary for how to add automated tests. The contributor task document is worth reading.

[edit] ps. I would recommend you also get a browser plugin from http://dreditor.org that many of us contributors use. (one of the things it has is a button insert tasks, that I used to insert the tasks template into the summary)

mgifford’s picture

@YesCT - sorry never got back to you in #109. Yes.. On D8, I'm always trying to work against head. Thanks for the link (which will need to be updated).

They come up with DREditor pretty easily. That's how generally I get to SimplyTest.me from the issue queue.

@adci_contributor - if you need help writing tests for this, I'm sure @YesCT can help direct you to the right person.

BarisW’s picture

I would be happy to write tests (or at least, try to write them). I always find it hard to determine whát to test. Can someone describe which tests are needed?

mgifford’s picture

Issue summary:View changes
mgifford’s picture

Well, the order doesn't seem to be something that we can test for...

We might be able to test if Drupal.announce() is fired.. To identify that change.

@YesCT - any other ideas?

idebr’s picture

Status:Needs review» Needs work
Issue tags:+Needs reroll

Patch no longer applies after #1663210: Clean up css in the User module was committed:

error: patch failed: core/modules/user/css/user.module.css:1
error: core/modules/user/css/user.module.css: patch does not apply
error: patch failed: core/modules/user/user.js:13
error: core/modules/user/user.js: patch does not apply

polaki_viswanath’s picture

Status:Needs work» Needs review
StatusFileSize
new5.26 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,254 pass(es).
[ View ]

Rerolled