Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#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

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 3 years ago.
Add screen reader text for full screen TinyMCE buttons
26555.2.patch (732 bytes) - added by joedolson 3 years ago.
Updated patch for TinyMCE 4

Download all attachments as: .zip

Change History (16)

#1 @SergeyBiryukov
3 years ago

  • Description modified (diff)

#2 @joedolson
3 years 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.

#3 @joedolson
3 years ago

  • Keywords has-patch added

3 years ago

Add screen reader text for full screen TinyMCE buttons

#4 @nacin
3 years 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

#5 @azaozz
3 years 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?

#6 @joedolson
3 years 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.

#7 follow-up: @joedolson
3 years 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.

3 years ago

Updated patch for TinyMCE 4

#8 in reply to: ↑ 7 @azaozz
3 years 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.

#9 @joedolson
3 years 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.

#10 @azaozz
3 years 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

#11 @joedolson
3 years 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.

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

3 years ago

#13 @nacin
3 years ago

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

#14 @azaozz
3 years 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.