WordPress.org

Make WordPress Core

#26555 closed defect (bug) (fixed)

Remove title attributes: class-wp-editor.php

Reported by: joedolson Owned by: azaozz
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: TinyMCE Keywords: has-patch
Focuses: accessibility Cc:

Description (last modified by SergeyBiryukov)

Related: #24766

// class-wp-editor.php
wp_fullscreen_html()

There is one title attribute here on what I think is a TinyMCE button. If that looks like a button, it should actually be a button.

Attachments (2)

26555.patch (776 bytes) - added by joedolson 21 months ago.
Add screen reader text for full screen TinyMCE buttons
26555.2.patch (732 bytes) - added by joedolson 19 months ago.
Updated patch for TinyMCE 4

Download all attachments as: .zip

Change History (16)

comment:1 @SergeyBiryukov21 months ago

  • Description modified (diff)

comment:2 @joedolson21 months ago

In this case, I left the title attribute - for now. Would value a 2nd opinion. Because that's the only method of labeling the button, I think it's necessary for discoverability of functionality. It would probably be improved by a jQuery driven method that was also keyboard activated. (The fullscreen editor has some notable keyboard issues, however, see #25111, which should be handled separately.)

This patch introduces screen-reader-text in the button that will be available to screen reader users, but there's still no discoverability of the button functions for keyboard users.

comment:3 @joedolson21 months ago

  • Keywords has-patch added

@joedolson21 months ago

Add screen reader text for full screen TinyMCE buttons

comment:4 @nacin19 months ago

  • Component changed from Accessibility to TinyMCE
  • Focuses accessibility added
  • Milestone changed from Awaiting Review to 3.9
  • Owner set to azaozz
  • Status changed from new to assigned

comment:5 @azaozz19 months ago

The markup has changed there: now it is actually a button :)

Comparing it to the new TinyMCE buttons, they use aria-labeledby="the-element-id" aria-label="Distraction Free Writing". Should we make the DFW buttons the same instead of using off-screen spans?

comment:6 @joedolson19 months ago

That sounds good - I hope that it's actually aria-labelledby; it's spelled with two ells.

I'll update the patch and test.

comment:7 follow-up: @joedolson19 months ago

I can't see a purpose to using aria-labelledby in this context. Looking at the TinyMCE implementation, it's back-referencing itself for a label -- the aria-labelledby (which is misspelled in TinyMCE, in fact), references it's own ID. However, the button still has no text content, so screen readers should fall back to using the aria-label attribute.

I don't think this is harmful, but it does seem pointless. (Especially given that aria-labeledby is spelled incorrectly, and probably won't be picked up at all.

I'm proposing a patch that simply uses aria-label; unless somebody chimes in and says that aria-labelledby is necessary here. But generally aria-labelledby is dependent on visual text that as control is being associated with.

I'm also removing role="presentation" -- assigning that role takes controls out of focusable patterns; I'm not clear why those would be there? I think this is a problem in the TinyMCE controls, as well, as I can't move keyboard focus to any of those controls.

The patch I'm attaching now is somewhat moot at the moment; it may fix issues for screen readers, but since I can't launch the DFW interface using the keyboard or navigate any part of the DFW interface using the keyboard, I can't test it.

See #25111 for my previous report about keyboard focus within the DFW model; I'll create a separate report for keyboard access to the TinyMCE controls.

@joedolson19 months ago

Updated patch for TinyMCE 4

comment:8 in reply to: ↑ 7 @azaozz19 months ago

Replying to joedolson:

I can't see a purpose to using aria-labelledby in this context.

This may be a back-compat for something, especially with the single l. If I remember right, the spelling of aria-labelledby was changed from a single to double l relatively recently?

TinyMCE is actually quite good with accessibility. To access the toolbar you'll need to press Alt + F10 while inside the editor. Our buttons don't seem to have :focus styling but can add that (in another ticket?) if necessary.

comment:9 @joedolson19 months ago

Aria-labelledby has never been spelled with one l - there was some belief that IE would accept that at one point, but that came about because IE was just doing a good job of guessing what the label was.

I added a ticket for the focus state earlier; #26932

Is the alt+f10 shortcut exposed anywhere? I never became aware of it, and I'm not sure whether it's all that meaningful if its not exposed to the user.

I'm not at a browser, but I'll look again when I am.

comment:10 @azaozz19 months ago

Is the alt+f10 shortcut exposed anywhere?

I believe it's read by screenreaders on the first editor focus. More info: http://www.tinymce.com/wiki.php/TinyMCE3x:Accessibility

comment:11 @joedolson19 months ago

Ah - I was worrying about keyboard accessibility for sighted users, not for screen reader users. It's a different group of people with disabilities; largely people with mobility impairments such as cerebral palsy, multiple sclerosis, etc., that impair fine motor skills.

For that audience, if there's a trigger, it needs to be exposed visually in some way or be available intuitively, as by tabbing. This is something we should work out; it doesn't have to be right in the middle of the interface, but it should be discoverable in some way.

comment:12 @ircbot17 months ago

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

comment:13 @nacin17 months ago

azaozz is going to update the attributes to what the TinyMCE button markup is now.

comment:14 @azaozz17 months ago

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

In 27766:

Add aria-label to the DFW buttons, props joedolson, fixes #26555

Note: See TracTickets for help on using tickets.