WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#27553 closed defect (bug) (fixed)

Make WP editor toggle focusable

Reported by: joedolson Owned by: azaozz
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.3
Component: Editor Keywords: has-patch
Focuses: accessibility Cc:

Description

Adds href="#" to the Text/Visual editor toggle so that they can receive keyboard focus natively. Edits CSS to change "float:right" to "display:inline-block" and changes code order of tabs so that tab order flows naturally in sequence when tabbing.

Attachments (5)

27553.patch (49.1 KB) - added by joedolson 5 years ago.
Add keyboard focus to WP editor toggles
27553.2.patch (1.6 KB) - added by iseulde 5 years ago.
27553.3.buttons.patch (3.0 KB) - added by afercia 5 years ago.
with buttons
dont-move-focus-from-title-to-post.patch (917 bytes) - added by afercia 5 years ago.
removes moving focus from Title to the editor when tabbing forwards
27553.diff (394 bytes) - added by jorbin 5 years ago.

Download all attachments as: .zip

Change History (38)

@joedolson
5 years ago

Add keyboard focus to WP editor toggles

#1 @joedolson
5 years ago

  • Keywords has-patch needs-testing added

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


5 years ago

#3 @nacin
5 years ago

Thanks joedolson. There's no need to patch minified files.

#4 @joedolson
5 years ago

Whoops; hadn't intended to include that. Sorry!

@iseulde
5 years ago

#5 @iseulde
5 years ago

  • Keywords needs-testing removed

Refreshed joedolson's patch:

  • Removes patched minified files.
  • Adds return false; so the link doesn't add # tot the URL.
  • Adds float: left; because the tabs are switched.
  • Adds text-decoration: none;.

#6 @DrewAPicture
5 years ago

Just adding a note to make RTL is covered here.

#7 @johnbillion
5 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Version changed from trunk to 3.3

Is there a reason for switching the order of the links in the DOM?

#8 @joedolson
5 years ago

I changed the DOM order so it would match the visible keyboard focus; but whether it's a CSS change or a DOM change I really don't care, as long as the visible focus moves in sequence.

#9 @GaryJ
5 years ago

Can the editor accept <button> instead of <a> for the ...buttons? If so, then no need for fake URLS and return false to stop native behaviour, but it still becomes focusable, more semantic, properly disable-able etc.

#10 @joedolson
5 years ago

It's not a drop-in solution; we'd need to override the button behavior and do a fair amount of restyling. It's a good idea, though - this kind of behavior is more appropriate to a button than an anchor.

Fixing the anchor was a nice quick and easy option.

#11 @sharonaustin
5 years ago

I did try both patches, neither worked. Joe, I sent you a password to get into the test site if you want to look around--maybe I did something wrong.

http://red-hound.com/trunk29747/trunk/src/?page_id=75

Thanks to all for the work being done--it's greatly appreciated.

#12 follow-ups: @afercia
5 years ago

New attached patch tries to use buttons.
I can't test in IE 9+ so please I need some feedback :)
There are still some issues:

  • maybe add some "screen-reader-text" to the buttons text, or a describedby to give context to "Visual" and "Text"
  • in IE (at least IE 8) when you activate the buttons, their text shifts 1 pixel down and right, this can be fixed but it's not so easy, the only way I'm aware of at the moment requires as first step to give buttons a fixed width... definitely not good for translations (unpredictable text length). Some CSS-fu stronger than mine is welcome, otherwise I'd recommend to live with the 1 pixel shift
  • when switching between Visual and Text editor, focus behaviour is inconsistent between browsers but this probably needs a separate ticket

@afercia
5 years ago

with buttons

#13 in reply to: ↑ 12 @sharonaustin
5 years ago

Replying to afercia:

New attached patch tries to use buttons.
I can't test in IE 9+ so please I need some feedback :)
There are still some issues:

  • maybe add some "screen-reader-text" to the buttons text, or a describedby to give context to "Visual" and "Text"
  • in IE (at least IE 8) when you activate the buttons, their text shifts 1 pixel down and right, this can be fixed but it's not so easy, the only way I'm aware of at the moment requires as first step to give buttons a fixed width... definitely not good for translations (unpredictable text length). Some CSS-fu stronger than mine is welcome, otherwise I'd recommend to live with the 1 pixel shift
  • when switching between Visual and Text editor, focus behaviour is inconsistent between browsers but this probably needs a separate ticket

I am liking the button idea more and more, between the button idea that Gary, Joe, you, and others are recommending; this feels like a good path to follow. I'll set up a place to test your patch (I'm slow, so it will take a while), but I would most certainly welcome more testers to join in.

This ticket was mentioned in IRC in #wordpress-ui by _Redd. View the logs.


5 years ago

#15 in reply to: ↑ 12 @sharonaustin
5 years ago

Replying to afercia:

New attached patch tries to use buttons.
I can't test in IE 9+ so please I need some feedback :)
There are still some issues:

  • maybe add some "screen-reader-text" to the buttons text, or a describedby to give context to "Visual" and "Text"
  • in IE (at least IE 8) when you activate the buttons, their text shifts 1 pixel down and right, this can be fixed but it's not so easy, the only way I'm aware of at the moment requires as first step to give buttons a fixed width... definitely not good for translations (unpredictable text length). Some CSS-fu stronger than mine is welcome, otherwise I'd recommend to live with the 1 pixel shift
  • when switching between Visual and Text editor, focus behaviour is inconsistent between browsers but this probably needs a separate ticket

This patch was tested here: http://red-hound.com/trunk29764/trunk/src/ and has real success! Woot! Woot! afercia is right, more work is required, because it works but requires a specific sequence (TBD)...but but it WORKS! Major thanks to afercia, Joe, Gary, avryl, nacin.

The button concept seems to be the kernel of the solution.

#16 @sharonaustin
5 years ago

More follow-up. Yesterday, because of afercia's insight gained from the IRC chat, https://irclogs.wordpress.org/chanlog.php?channel=wordpress-ui&day=2014-09-24&sort=asc
(#wordpress-ui, September 24 2014, time frame 20:24 to 20:31) I understood that javascript comes into play as part of the problem as to why the Text/Visual editor tabs don't receive focus.

To even begin to test "around" the problem, one must first launch from the TinyMCE editor.

With this understanding, I went back and re-tested joedolson/avryl's patch using the same technique of trying to launch from inside the TinyMCE editor, and found that these patches ALSO worked. Also, joedolson's code to ensure DOM order matched visual order was effective here.

  1. All three patches work.
  2. Thanks to afercia's insight, we now have more tools to fight the problem.
  3. For the next steps, we need accessibility folks more expert than me to be involved in guiding "how" focus should be controlled as one progresses through the interface. See the 20:53 minute of the IRC chat for context.
  4. I have yet to continue the promised testing on different browsers. Any additional testing is most certainly welcome.

Great thanks again to all involved, special thanks to the patch makers.

#17 follow-up: @afercia
5 years ago

In /wp-admin/js/post.js there's some code commented with "This code is meant to allow tabbing from Title to Post content" which basically does one thing:
when you're in the "Title" field and then you tab forwards, focus is moved directly inside the editor area, whether it's TinyMCE or the Text editor, skipping all what's in between the "Title" field and the editor.

While this *may* be useful for sighted keyboard users (they save a few key presses, say 1 second?), it's a very critical experience for screen reader users: when they tab forwards, they get that right after the title there's the editor. But when they tab backwards, they get many more elements that "weren't there before".

Moreover, TinyMCE and the Text editor differ in the number of focusable elements you have to tab through while tabbing backwards from the editor to the "Add media" button:

  • with TinyMCE you have to press Shift + Tab just once
  • with the Text editor you have to press Shift + Tab 14 times to skip all the quicktags buttons and the "toggle fullscreen" button

No information is provided anywhere to screen reader users about this difference. Plugins and themes may add additional buttons too.

I guess this is one of the most confusing and frustrating experiences for screen reader users. Quoting a post by @sharonaustin will help to understand this far better than trying to explain with my poor English:

We don’t know why this happens.
...
Certain elements aren’t available by keyboard when tabbing forward, and by extension, aren’t available to assistive technology – the strange thing is, they’re accessible if you tab backwards….WHY?

http://reddhoundhowling.wordpress.com/

Attached patch is for testing purposes and it just removes the javascript part responsible for moving the focus, thus restoring a natural tab flow and order.
It would be really helpful to have as much people as possible testing this, especially accessibility experts and users.
Any feedback more than welcome.

@afercia
5 years ago

removes moving focus from Title to the editor when tabbing forwards

#18 in reply to: ↑ 17 @sharonaustin
5 years ago

Replying to afercia:

Attached patch is for testing purposes and it just removes the javascript part responsible for moving the focus, thus restoring a natural tab flow and order.
It would be really helpful to have as much people as possible testing this, especially accessibility experts and users.

@afercia -- We test on Mondays. Usually, there is at least one experienced screen-reader user that is also present. I'll have the latest patch in the test site by then, and we can test for the different browsers and with screen readers at that time. 17:00UTC on Mondays, IRC #wordpress-ui. By testing in a public channel we'll be able to share any knowledge gained from our efforts. If, for some reason I won't be able to attend personally (I'm never sure if I'll be able to join in on scheduled meetings), I'll send logins to you and any one who wants to participate so everyone can get in there and test. Thanks again for the work on this--it's greatly appreciated.

#19 follow-ups: @joedolson
5 years ago

I definitely agree that there should be a much better tab order on the post editing page; but it's not quite that simple. Tabbing directly from the title field to the content field doesn't just save a few clicks; it can actually save literally dozens of tab keypresses. If the user is using the HTML editor and has just the basic set of quicktags enabled, it's only about a dozen of keypresses; but if they're using the visual editor and have additional buttons enabled, tabbing through could easily take 40 or 50 additional keypresses.

I think that the automatic habit of tabbing directly from the title field into content is pretty engrained in users, so this would be a major inconvenience for a lot of people.

So, while I agree that we need a better solution, I don't think that simply eliminating this tab shortcut is going to be the right solution.

There's already a keyboard trigger for entering the toolbar for the visual editor, it would be helpful if that also worked to enter the toolbar in HTML mode.

However, this should really be raised as a separate ticket; it's well out of scope for this ticket.

#20 in reply to: ↑ 19 @sharonaustin
5 years ago

Replying to joedolson:

I think that the automatic habit of tabbing directly from the title field into content is pretty engrained in users, so this would be a major inconvenience for a lot of people.

So, while I agree that we need a better solution, I don't think that simply eliminating this tab shortcut is going to be the right solution.

What do you think of the idea of building a version of a "Skip" link, similar in functionality to the "Skip Navigation" links that we employ in the Dashboard? The purpose would be to give a screen reader user the OPTION of skipping the additional buttons, going directly to the editor.

In other words, "keep" the connection, but make it "optional" to use.

#21 in reply to: ↑ 19 @afercia
5 years ago

Replying to joedolson:

Tabbing directly from the title field to the content field doesn't just save a few clicks; it can actually save literally dozens of tab keypresses. If the user is using the HTML editor and has just the basic set of quicktags enabled, it's only about a dozen of keypresses; but if they're using the visual editor and have additional buttons enabled, tabbing through could easily take 40 or 50 additional keypresses.

That's true for the HTML (Text) editor with quicktags buttons enabled, as I said there are 14 key presses in the default configuration. Not true for the Visual editor because the buttons toolbar is not focusable by default, it's completely skipped and to give focus to that toolbar you have to press Alt + F10 once you're in the TinyMCE editing area. As far as I know that's the only way to give focus to the TinyMCE buttons toolbar, unless I'm missing something. Thus, in a default configuration there would be just 5 key presses and making the toggle tabs focusable there would be 7 key presses.

... we need a better solution

There was a project aimed to review this area https://make.wordpress.org/ui/tag/ceux/ but it's been put "on hold" and it's currently inactive. I noticed some interesting ideas there.
I tend to think that moving focus without informing users why and where the focus is moved and what has been skipped it's usually a bad idea. A natural tab order is a better option in most of the cases and if there are problems, they usually come from a lack of a logical sequence in the markup. CEUX project noticed this and you can see in some mockups attached in the project page they proposed to unify the Title field and the editor, removing anything in between.

There's already a keyboard trigger for entering the toolbar for the visual editor, it would be helpful if that also worked to enter the toolbar in HTML mode.

I agree the "tabbing experience" in the Visual and Text editors should match as much as possible. Not sure forcing the quicktags toolbar out of the natural tab order would be the right solution. Giving the option of skipping as proposed by @sharonaustin would be probably a better choice.

However, this should really be raised as a separate ticket; it's well out of scope for this ticket.

I agree. Will create a new ticket depending on the feedback received by the a11y mailing list group.

#22 @sharonaustin
5 years ago

Been thinking about this a little more; I think we have some golden opportunities for testing even now.

First, as I understand it, both <a> elements and <button> elements are HTML DOM objects, which is good because screen readers work off DOM elements. The thing I'm not sure about is whether screen readers treat the <a> element and the <button> element the same way. I think that this alone is worth testing, to make sure we're not working off any assumptions that aren't valid. Testing on different browsers with major screen readers (JAWS, NVDA, Window-Eyes, Voice-Over) would give us a better handle on whether there is a difference between how a screen reader treats an <a> element versus a <button> element.

Second, out of my ignorance, I've been assuming that if we understand the tabbing order, we could accurately extrapolate "how" assistive technology works based on that. After some background reading from the input of @bramd for ticket #29371 and this link http://doccenter.freedomscientific.com/doccenter/doccenter/rs25c51746a0cc/2012-12-12_aria/02_ariacontrols.htm I understand that this may not be the case--arrow keys are involved, and may or may not follow tab order.

There's a lot of baseline knowledge we can gain simply from testing what we have right now--two patches using <a> element, a third patch using <button> element, seeing if there is any difference between browsers, seeing if there is any difference between screen readers, seeing if any of them automatically force a screen reader into forms mode.

Any input is welcome, as is testing.

Thanks again so much to all involved.

#23 follow-up: @joedolson
5 years ago

Hi, Sharon - I don't think any of that specific testing is necessary. Button and anchor elements do behave differently in assistive technology, and this is a long-standing, normal behavior that's well understood. The question in this ticket is not whether one option or another will work, but which is the most appropriate choice. Either will work just fine, but have a different activation behavior in a screen reader.

Using buttons simply requires more effort to re-style the controls, re-writing the prior set up.

Regarding DOM objects -- all elements on any page are DOM objects. The DOM is the model of objects in a document; nothing can be on the page and not be part of the DOM.

This ticket was mentioned in IRC in #wordpress-ui by _Redd. View the logs.


5 years ago

#25 in reply to: ↑ 23 ; follow-up: @sharonaustin
5 years ago

Replying to joedolson:

Definitely appreciate the feedback, Joe! Especially knowing how busy you are....Thank you so much.

  1. Regarding this:

    Regarding DOM objects -- all elements on any page are DOM objects. The DOM is the model of objects in a document; nothing can be on the page and not be part of the DOM.

I worded my earlier statement about DOM objects poorly in the context of this ticket; I understand that all elements on a page are DOM objects but--and I am making this next statement in all uncertainty--I don't think screen readers access all the DOM elements in all modes. Take the example of a "div"--aren't they "blind" to screen readers unless they receive special handling? (By all means, please feel free to correct me on that, sir). A specific example I have in mind is the current fad of styling divs to look like buttons, turning these false buttons into an accessibility barrier (if the div doesn't get states for focus, etc. ).

Another example. Yesterday, before I read this response, I had been testing with NVDA on the first two patches, yours and avyrl's, in one of my test sites. It turns out NVDA would read out loud the text that was already in place in the "Visual" Editor screen, but not the "Text" editor screen, of the TinyMCE. I have to assume at some level that the text in the "Text Editor" is a DOM element, but NVDA was blind to text already in the "Text Editor" screen. I don't know why.

At any rate, I definitely appreciate the feedback, but as I think you said before, this conversation is for another ticket, and I think me going off on tangents is problematic here, so no more testing from me unless requested. I'll watch with very great interest any further developments on this particular ticket. Let me know what I can do.

Take care, Joe, and thanks for everything you do.

#26 in reply to: ↑ 25 @afercia
5 years ago

Replying to sharonaustin:

It turns out NVDA would read out loud the text that was already in place in the "Visual" Editor screen, but not the "Text" editor screen, of the TinyMCE.

I created a ticket for this, see #29815

#27 follow-up: @joedolson
5 years ago

Patch #3 by afercia appears to work quite well. There are outstanding issues surrounding tab order, but this are long-standing and not relevant to this specific patch and issue; that needs to be raised separately.

@afercia - You said that you would be raising that ticket separately; are you still intending to do that?

#28 in reply to: ↑ 27 @afercia
5 years ago

Replying to joedolson:

@afercia - You said that you would be raising that ticket separately; are you still intending to do that?

hi @joedolson, I was waiting for some feedback from the accessibility Monday testing meeting but the testing session was postponed.
Anyway, I've just created #29838 trying to do my best with my poor English.

This ticket was mentioned in IRC in #wordpress-ui by _Redd. View the logs.


5 years ago

#30 @azaozz
5 years ago

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

In 30002:

Editor: use <button> instead of <a> for the Visual/Text buttons, make them focusable. Props afercia, fixes #27553

@jorbin
5 years ago

#31 @jorbin
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

we should have run autoprefixer on this since it added box-sizing: content-box

FF before 29 and Safari before 5.1 require the prefix.

Above patch fixes that.

#32 @jorbin
5 years ago

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

In 30070:

Autoprefix box sizing

Results are from running grunt autoprefix. Needed for FF before 29 and Safari before 5.1.

fixes #27553

#33 @SergeyBiryukov
5 years ago

  • Milestone changed from Future Release to 4.1
Note: See TracTickets for help on using tickets.