Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#26602 closed defect (bug) (fixed)

Insufficient information for screen readers in themes.php

Reported by: grahamarmfield's profile grahamarmfield Owned by: nacin's profile nacin
Milestone: 3.8.1 Priority: normal
Severity: normal Version: 3.9
Component: Accessibility Keywords: has-patch commit needs-testing reporter-feedback
Focuses: Cc:

Description

On the new Themes page in trunk, it is now possible to tab to each of the themes within the list of themes found. However for screen readers there may be no audible feedback to tell the user which theme has focus.

This is because it is a <div> that is getting focus each time (courtesy of a tabindex="0"). But the <div> itself doesn't have any descriptive text to let assistive technology know what it represents.

Screen readers have a go - seemingly pulling in some nearby text. In NVDA this happens to be the theme name, but in JAWS all you get is "Theme Details" for each theme. Notification of which theme is currently active is also missing although screen readers do find the Customize button.

Appropriate ARIA attributes will need to be attached to the <div> that gets focus to identify which theme it represents.

Attachments (7)

26602.patch (2.2 KB) - added by joedolson 11 years ago.
Communicate theme name and action to screen readers.
26602.1.patch (1.4 KB) - added by joedolson 11 years ago.
Corrects error in previous patch.
26602.diff (1.4 KB) - added by jorbin 11 years ago.
26602.2.diff (1.4 KB) - added by nacin 11 years ago.
Fixed typo closing class attribute.
source-showingsome-changes.gif (36.7 KB) - added by GrahamArmfield 11 years ago.
Source view showing changes in place.
inspector-showing-absence-of-attributes.gif (26.7 KB) - added by GrahamArmfield 11 years ago.
View from Chrome Inspector - the attributes are not showing.
26602.3.diff (3.0 KB) - added by nacin 11 years ago.

Download all attachments as: .zip

Change History (30)

#1 @toscho
11 years ago

  • Cc info@… added

#2 @jorbin
11 years ago

  • Milestone changed from Awaiting Review to 3.8.1

#3 @joedolson
11 years ago

I think that a patch for this somewhat depends on what happens with the keyboard focus patch (#26527); I can provide a patch that assumes the current behavior (navigation by keyboard through the <div> elements), but an actual commit to this really depends on choices made in keyboard navigation. If, instead of navigating to the <div> element, navigation passes from action items (Activate & Customize links), then instead this needs to use aria-describedby or screen-reader-hidden to indicate which theme the link references.

Regardless, right now, my experience navigating by keyboard in Chrome or Firefox is that focus is moving from each individual Activate & Customize anchor element within the themes page, which is not the behavior described in this ticket.

A second opinion would be appreciated.

Last edited 11 years ago by joedolson (previous) (diff)

#4 @joedolson
11 years ago

  • Keywords 2nd-opinion added

@joedolson
11 years ago

Communicate theme name and action to screen readers.

@joedolson
11 years ago

Corrects error in previous patch.

#5 @joedolson
11 years ago

Attached patch adds aria-describedby getting theme name and action performed; second patch corrects error in action reference in first patch.

#6 @grahamarmfield
11 years ago

Do these patches need to be applied on top of 26527.9?

If so, I'm getting 'rejected hunks' applying both the 26602 patches.

#7 @sharonaustin
11 years ago

  • Cc sharonaustin added

This ticket was mentioned in IRC in #wordpress-dev by jorbin_work. View the logs.


11 years ago

#9 follow-up: @jorbin
11 years ago

  • Keywords needs-testing has-patch added

26527 is now fixed in trunk and Joedolson's patch applies cleanly.

@grahamarmfield can you test this?

#10 in reply to: ↑ 9 ; follow-up: @GrahamArmfield
11 years ago

Replying to jorbin:

26527 is now fixed in trunk and Joedolson's patch applies cleanly.

@grahamarmfield can you test this?

Unfortunately I'm still getting rejected hunks when I try to apply Joe Dolson's patches. I'm applying on to a fresh update of trunk - as at 09:30 GMT 10th Jan.

@jorbin
11 years ago

#11 in reply to: ↑ 10 ; follow-up: @jorbin
11 years ago

Replying to GrahamArmfield:

Replying to jorbin:

26527 is now fixed in trunk and Joedolson's patch applies cleanly.

@grahamarmfield can you test this?

Unfortunately I'm still getting rejected hunks when I try to apply Joe Dolson's patches. I'm applying on to a fresh update of trunk - as at 09:30 GMT 10th Jan.

I updated joe's most recent patch and should work fine now.

#12 in reply to: ↑ 11 @GrahamArmfield
11 years ago

Replying to jorbin:

I updated joe's most recent patch and should work fine now.

OK, patch successfully applied so thanks for that.

I tested this using NVDA running with Firefox on Windows 7 machine. When tabbing forward through the themes, the screen reader voices -

(tab)
clickable
Theme Details active
[Theme name] heading level 3 Customize
(tab)
Customize link
(tab)
clickable
Theme Details
Twenty Eleven
(tab)
clickable
Theme Details
Twenty Fourteen
(tab)
...
etc...

So, so far so good - although the Live Preview and Activate links should ideally also contain the theme name to provide context.

But when reverse (shift) tabbing NVDA voices:

(shift tab)
clickable
Live Preview link
(shift tab)
Activate link
(shift tab)
-- silence -- (NVDA doesn't voice anything)
(shift tab)
clickable
Live Preview visited link
(shift tab)
Activate link
(shift tab)
-- silence --

The silences are when focus is resting on the Theme details 'link'. Because of this, NVDA users get no feedback about which theme they are exploring when they are reverse tabbing. Also it seems odd that the Live preview and activate links are part of the tab order when reverse tabbing but not when forward tabbing. But I think that's a question with the general keyboard accessibility rather than screen reader.

In IE10 with NVDA I get substantially the same result, except that when forward tabbing it reads

(tab)
Theme Details
[Theme name] heading level 3
activate
link live preview
(tab)
Theme Details
[Theme name] heading level 3
activate
link live preview

so it's reading the links as part of the theme name.

When reverse tabbing I get the same results as with Firefox most of the time, but just occasionally I also hear "Theme details" instead of a silence.

I'll be testing with JAWS also and will comment separately on that.

#13 @GrahamArmfield
11 years ago

After trying with JAWS I'm not convinced that the patch is applying successfully in my dev area. It seems to apply OK and the new code elements are present in themes.php, but the aria-describedby bits are not seemingly appearing in the browser. I'm investigating, but for now best to treat my previous post as provisional only.

@nacin
11 years ago

Fixed typo closing class attribute.

#14 follow-up: @nacin
11 years ago

GrahamArmfield, can you try 26602.2.diff? There was a typo in the patch.

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


11 years ago

#16 @nacin
11 years ago

  • Keywords commit added; 2nd-opinion removed

Marking for tentative commit. Needs to be tested.

#17 in reply to: ↑ 14 @GrahamArmfield
11 years ago

  • Keywords reporter-feedback added

Replying to nacin:

GrahamArmfield, can you try 26602.2.diff? There was a typo in the patch.

I cannot get this patch to do what it's supposed to do. Thought it might have been my mistake the first time.

I've applied the patch and can see the revised code within themes.php in my test area. However when running the WP admin and navigating to the Themes screen the changes are not present in the file.

I have attached to screenshots - the first shows the source of themes.php with a couple of the changes highlighted. The second screenshot shows a view from Chrome Inspector where you can plainly see that new attributes are not present.

I know that this is the correct file because when I do put extra characters in the tmpl-theme section below (line 296 for example), these do get reported out to the screen.

Is there something that strips out the extra attributes later on, or in any javascript?

I so want this patch to work, but it's not happening at the moment.

@GrahamArmfield
11 years ago

Source view showing changes in place.

@GrahamArmfield
11 years ago

View from Chrome Inspector - the attributes are not showing.

#18 @nacin
11 years ago

  • Milestone changed from 3.8.1 to 3.8.2

I don't think the theme ID is safe for an attribute either, FWIW. Needs escaping.

@nacin
11 years ago

#19 @GrahamArmfield
11 years ago

This patch is a definite improvement on what was there before. Theme Details and Theme name are mentioned by both NVDA on Firefox and JAWS on IE10. Also, "Active" is voiced for the currently active theme.

There is still an issue with reverse tabbing in NVDA - the Live Preview and Activate buttons are voiced OK although they do need some extra context. But when reverse tabbing, I'm still getting silences when I land on the Theme Details 'link'.

#20 @GrahamArmfield
11 years ago

I know it's not up to me, but my view would be that this patch is ready to go. The issue described may be an issue with NVDA rather than themes.php - it'll need a bit of investigation.

#21 @nacin
11 years ago

  • Milestone changed from 3.8.2 to 3.8.1

#22 @nacin
11 years ago

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

In 27012:

Themes screen: Add aria-describedby attributes to provide better information to screen readers.

props joedolson for initial patch.
fixes #26602 for trunk.

#23 @nacin
11 years ago

In 27013:

Themes screen: Add aria-describedby attributes to provide better information to screen readers.

Merges [27012] to the 3.8 branch.

props joedolson for initial patch.
fixes #26602.

Note: See TracTickets for help on using tickets.