Make WordPress Core

Opened 19 months ago

Closed 11 months ago

Last modified 8 months ago

#54483 closed defect (bug) (fixed)

On logout input fields have aria described login_error

Reported by: patrickgroot's profile patrickgroot Owned by: joedolson's profile joedolson
Milestone: 6.1 Priority: normal
Severity: trivial Version: 5.8.2
Component: Login and Registration Keywords: has-patch commit add-to-field-guide
Focuses: ui, accessibility Cc:

Description

Hi Team,

Currently i'm trying to smooth out the UI a bit while developing a plugin that add some extra functionality to the login and registration pages of WordPress.

One thing i noticed is while you're logged-in into your WordPress site is that when you're logging out the input fields from the login form have a aria-describedby="login_error" what doesn't make sense when you're just logged-out.

URL for testing: mydomain.com/wp-login.php?loggedout=true

Attachments (4)

54483.diff (3.8 MB) - added by rollybueno 16 months ago.
Created patch.
54483.2.diff (1.6 KB) - added by rollybueno 16 months ago.
Created correct patch file. The first one that I uploaded earlier was created using wrong method, thus having 4MB size. It can be disregarded.
54483.3.diff (2.3 KB) - added by afercia 15 months ago.
54483.4.diff (2.4 KB) - added by afercia 14 months ago.

Change History (21)

#1 @patrickgroot
19 months ago

Potential/proposal fix

Original file: wp-login.php

<?php
if ( $errors->has_errors() ) {
    $aria_describedby_error = ' aria-describedby="login_error"';
} else {
    $aria_describedby_error = '';
}

Changed line 1370:

<?php
if ( $errors->has_errors() && empty( $_GET['loggedout'] ) ) {
    $aria_describedby_error = ' aria-describedby="login_error"';
} else {
    $aria_describedby_error = '';
}
Last edited 19 months ago by patrickgroot (previous) (diff)

This ticket was mentioned in PR #1926 on WordPress/wordpress-develop by pgroot91.


19 months ago
#2

  • Keywords has-patch added

Adds an extra check preventing the input fields from having an aria described of login_errors while you just loggedout of your website.

Trac ticket: 54483

This ticket was mentioned in Slack in #accessibility by alexstine. View the logs.


18 months ago

#4 @alexstine
18 months ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to 6.0
  • Owner set to alexstine
  • Status changed from new to accepted

The Accessibility team thinks it may be best to have the successful logout message read to screen readers. The current success message is not connected via aria-describedby. I will work on a patch to test shortly.

Thanks.

@rollybueno
16 months ago

Created patch.

@rollybueno
16 months ago

Created correct patch file. The first one that I uploaded earlier was created using wrong method, thus having 4MB size. It can be disregarded.

#5 @rollybueno
16 months ago

  • Keywords has-patch added; needs-patch removed

Hi here,

I noticed that this ticket has no movement for 2 months so I have uploaded a new patch(54483.2.diff). I have updated the wp-login.php that:

  1. On user logged out, add specific value for the aria-describedby to the fields.
  2. On the login form message, I have applied the same value of the aria-describedby from the fields inside the message paragraph

@afercia
15 months ago

#6 @afercia
15 months ago

Given that the value of aria-describedby should point to the ID of an element present on the page, I'd like to propose a different approach in 54483.3.diff. This updated patch should cover the case of the login and logout.

Worth noting there are at least 3 more cases where input fields aren't connected to messages displayed on the page:

  • lost password: this page shows a default message and potential errors, however the input field is not connected to any description.
  • reset password: the password field is connected to the password strength meter, which is also an aria-live region.
  • register: this page shows a default message and potential errors, however the two input fields are not connected to any description.

I'm not sure these cases should be addressed in the context of this ticket though.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


14 months ago

#8 @ryokuhi
14 months ago

This ticket was reviewed today during the Accessibility Team's weekly bug-scrub.
Adding the needs-testing label, so that this ticket gets a little exposure.

@afercia
14 months ago

#9 @afercia
14 months ago

54483.4.diff is a refresh against latest trunk.

To test:

  • Logout from the admin.
  • Check both the username and password fields have an aria-describedby attribute that points to the success notice with id login-message.
  • Try to login by entering invalid credentials.
  • Check both the username and password fields have an aria-describedby attribute that points to the error notice with id login_error.

#10 @sabernhardt
14 months ago

  • Milestone changed from 6.0 to 6.1

#11 @sabernhardt
14 months ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


11 months ago

#13 @joedolson
11 months ago

  • Owner changed from alexstine to joedolson

#14 @joedolson
11 months ago

  • Keywords commit added; needs-testing removed

Tested and works as expected; marking for commit.

#15 @joedolson
11 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 53707:

Login: Explicitly associate errors with input fields.

Add an aria-describedby relationship between login email/password input fields and displayed login error messages.

Props patrickgroot, rollybueno, afercia.
Fixes #54483.

#16 @joedolson
11 months ago

In 53708:

Coding Standards: Missed space in code indentation in [53707].

Fixes code style issue from [53707].

Follow up to [53707].

See #54483.

#17 @milana_cap
8 months ago

  • Keywords add-to-field-guide added
Note: See TracTickets for help on using tickets.