Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#42632 closed defect (bug) (fixed)

Remove tabindex="-1" from login form header

Reported by: bamadesigner's profile bamadesigner Owned by: afercia's profile afercia
Milestone: 5.1 Priority: normal
Severity: normal Version:
Component: Login and Registration Keywords: has-patch needs-testing
Focuses: accessibility Cc:

Description

The h1 on the login form has an <a> with a tabindex attribute set to -1, which means that it cannot receive keyboard focus. This is an accessibility violation as it disable users who are navigating the page via keyboard from accessing the action.

To fix, all that has to be done is remove ' tabindex="-1"' from line 161 in wp-login.php:

<?php
<h1><a href="<?php echo esc_url( $login_header_url ); ?>" title="<?php echo esc_attr( $login_header_title ); ?>"><?php bloginfo( 'name' ); ?></a></h1>

Attachments (6)

ticket-42632.diff (634 bytes) - added by bamadesigner 7 years ago.
Patch for the bug
42632.patch (1.8 KB) - added by rishishah 7 years ago.
I have removed tabindex="-1" from all login and setup files.
42632.diff (1.8 KB) - added by jainnidhi 7 years ago.
for42632.diff (4.8 KB) - added by bamadesigner 7 years ago.
Patch for ticket 42632
42632.2.diff (4.1 KB) - added by afercia 7 years ago.
42632.3.diff (3.9 KB) - added by afercia 6 years ago.

Download all attachments as: .zip

Change History (25)

@bamadesigner
7 years ago

Patch for the bug

#1 @afercia
7 years ago

See #28674 and [28896].

Last edited 7 years ago by afercia (previous) (diff)

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


7 years ago

#3 @afercia
7 years ago

See also #42537 for other proposed improvements to the logo link.

#4 @afercia
7 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.0
  • Version 4.9 deleted

Discussed a bit this ticket during today's accessibility meeting. Seems the main argumentation in the original ticket #28674 was about the install / setup pages are actually a multi-step process and having to tab through the logo link at each step is a bit annoying.

However, on other pages like the login one, the logo is just removed from the tab order even if the username field receives the initial focus.

Similarly to the login page, the install / setup process is maybe one of the few cases where moving focus programmatically would make sense. There’s one, specific, task and focusing the first field would speed up the process. The task is mandatory; you don't really have any other options during the process.

For these reasons, we'd like to second @bamadesigner proposal and remove tabindex="-1" from all these pages. On the install / setup pages, the first focusable field should receive the initial focus and there's the HTML5 autofocus attribute which is worth trying.

@rishishah
7 years ago

I have removed tabindex="-1" from all login and setup files.

#5 @Presskopp
7 years ago

  • Keywords has-patch added; needs-patch removed

#6 @SergeyBiryukov
7 years ago

  • Keywords needs-patch added; has-patch removed

Per comment:4, removing tabindex="-1" on install pages is not enough, we should also make sure the first focusable field receives the initial focus.

@jainnidhi
7 years ago

#7 @jainnidhi
7 years ago

  • Keywords has-patch added; needs-patch removed

I've attached 42632.diff file which has removed tabindex="-1" from all login and setup files.

#8 @afercia
7 years ago

  • Keywords needs-refresh added

Thanks @jainnidhi, I see your patch is identical to the previous one. Apart from removing tabindex="-1", there's the need to make sure the first form field receive focus in all the install pages. See in a previous comment:

On the install / setup pages, the first focusable field should receive the initial focus and there's the HTML5 autofocus attribute which is worth trying.

@bamadesigner
7 years ago

Patch for ticket 42632

#9 @bamadesigner
7 years ago

Is this what you're looking for, @afercia?

for42632.diff

#10 @afercia
7 years ago

@bamadesigner thanks. Looking a bit into this, I think my previous comment was a bit confusing, sorry for that. Most of the first fields in the installation/setup screens already receive the initial focus via some JavaScript. It's done in a way that focus is set only for non-mobile devices to avoid to trigger the mobile on-screen keyboard. See for example https://core.trac.wordpress.org/browser/trunk/src/wp-admin/install.php?rev=42343#L410. We should make sure all the first focusable fields behave in the same way.

Seems to me the only field that doesn't get initial focus is the dbname one. For consistency, it should receive focus but only on non-mobile devices.

@afercia
7 years ago

#11 @afercia
7 years ago

  • Keywords needs-testing added; needs-refresh removed

42632.2.diff adds autofocus only to the dbname field, only for non-mobile devices. Some testing welcome.

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


6 years ago

#13 @afercia
6 years ago

  • Milestone changed from 5.0 to 4.9.9

Discussed during today's accessibility bug-scrub an agreed we'd like to propose this for 4.9.9 consideration.

#14 @pento
6 years ago

  • Milestone changed from 4.9.9 to 5.0.1

#15 @pento
6 years ago

  • Milestone changed from 5.0.1 to 5.0.2

#16 @pento
6 years ago

  • Milestone changed from 5.0.2 to 5.0.3

#17 @audrasjb
6 years ago

  • Milestone changed from 5.0.3 to 5.1

Hi,

Since 5.0.3 is going to be released in few days and as the ticket still needs review and commit, let's address this one in 5.1, coming next month.

@afercia
6 years ago

#18 @afercia
6 years ago

42632.3.diff refreshes the patch.

#19 @afercia
6 years ago

  • Owner set to afercia
  • Resolution set to fixed
  • Status changed from new to closed

In 44545:

Accessibility: Remove negative tabindex from the login, install, and setup pages header.

Props bamadesigner, rishishah, jainnidhi.
Fixes #42632.

Note: See TracTickets for help on using tickets.