Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#19178 closed defect (bug) (fixed)

wp-login.php uses <label> without the associated for

Reported by: ppaire's profile ppaire Owned by: nacin's profile nacin
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.3
Component: Accessibility Keywords: has-patch
Focuses: Cc:

Description

Screen readers for disabled users don't always work with the implicit labeling. Therefore wp-login.php labels should be changed to have the explicit FOR to match with associated field's ID.

Attachments (1)

wp-login.php.patch (3.2 KB) - added by ppaire 12 years ago.
Updated wp-login.php that includes the label with associated for's

Download all attachments as: .zip

Change History (9)

@ppaire
12 years ago

Updated wp-login.php that includes the label with associated for's

#1 @jorbin
12 years ago

  • Component changed from General to Accessibility
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.3

#2 @nacin
12 years ago

Looks great!

#wcphilly

#3 @nacin
12 years ago

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

In [19189]:

Add 'for' attributes to labels in wp-login.php for extra accessibility. props ppaire, fixes #19178.

#4 @azaozz
12 years ago

This is actually bad HTML. It (used to?) confuse some browsers resulting in DOM errors.

Which screen readers don't understand <label><input /></label>?

#5 follow-up: @ppaire
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I was going off of what is claimed in the WCAG 2 standards, not based on personal experience with screen readers.

http://www.w3.org/TR/WCAG20-TECHS/H44.html

They state in http://www.w3.org/TR/WCAG20-TECHS/F68.html that <label><input /></label> is invalid (granted, the standard was written years ago as they reference Jaws 7.10 in the documentation). Consequently it will fail validators that check for WCAG 2 compliance.

I'll move the </label> before the <input /> to fix the HTML unless you tell me to revert back to the original code.

#6 @nacin
12 years ago

This isn't bad HTML, AFAIK. <label><input /></label> is the implicit way of doing the association, but <label for=""> is then needed for older versions of IE to actually tie them together. Doing both, as far as I know, is no issue.

#7 @nacin
12 years ago

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

I'm re-closing this for now. A quick grep shows a few dozen similar constructs in core. If it *does* cause a problem, we'd need to fix it everywhere. But it's such common construction on the web I can't imagine that being the case.

#8 in reply to: ↑ 5 @azaozz
12 years ago

Replying to ppaire:

I was going off of what is claimed in the WCAG 2 standards, not based on personal experience with screen readers.

Yes, as far as I could find that standard is a bit outdated and doesn't reflect the current state of most screenreaders (including the build-in the browsers screenreaders).

Unfortunately as @nacin mentions having <label for="id"><input id="id" /></label> is somewhat common, that's why I didn't reopen the ticket. The problems we encountered was some time ago while working on the widgets screen: we had to remove the for="..." attributes from JS to make it work in all browsers.

Note: See TracTickets for help on using tickets.