Wp-calypso: Nav sidebar: tab navigation not working on Firefox

[p-jackson]

When you open the nav sidebar in the editor, the sidebar should behave like a modal. So clicking the dark background or pressing ESC should close the sidebar, and also importantly, pressing tab should only focus elements within the sidebar.

This is working on Chrome, but isn't working in Firefox.

My guess is that the call to .focus() to initially set the focus inside the sidebar isn't working on FF.

Part of #41298

[p-jackson]

The issue is that the only controls inside the sidebar (except for the close button) are <a>s, which aren't tabble in Firefox without special config. See this SO for details. tl;dr

1. In System Preferences → Keyboard, in the Shortcuts pane, check the “all controls” radio at the bottom.
2. In Firefox, type "about:config" in the URL bar. There is no accessibility.tabfocus preference on the mac, so you'll have to make one. Right click in the window, create a new "integer" pref, and set it to 7.

Step 1 being required seems fine, but step 2 is a surprise given it's not how Safari/Chrome/Edge work.

This causes a bug in Gutenberg's withConstrainedTabbing HoC because it assumes that links will always be tabble. The first and last controls in the sidebar need to be tabble for the logic of the HoC to work. I added a dummy <button> to the bottom of the sidebar to test this out and the tabbing was correctly constrained to the sidebar.

[andrewserong]

For whoever picks this one up, the sidebar lives in: https://github.com/automattic/wp-calypso/blob/HEAD/apps/full-site-editing/full-site-editing-plugin/wpcom-block-editor-nav-sidebar/src/components/nav-sidebar/index.tsx and the focus() call that Phil mentioned is at: https://github.com/automattic/wp-calypso/blob/HEAD/apps/full-site-editing/full-site-editing-plugin/wpcom-block-editor-nav-sidebar/src/components/nav-sidebar/index.tsx#L75

[autumnfjeld]

@andrewserong First, just want to clarify that I think this issue is about the tabbing, and _not_ about the “clicking the dark background or pressing ESC should close the sidebar” as I verified that closing the sidebar in this way works properly in Chrome, Safari, and Firefox on my Mac.

As @p-jackson mentioned in the comment above, the issue has to do with user preferences on a Mac. I did more digging on this.

I verified that the tab problem exists in both Safari and Firefox on my Mac, and according to the SO link that Phillip posted, the issue is system and/or browser preferences for keyboard navigation in _both_ Safari and Firefox on a Mac. Mac users must change their accessibility settings in order to take advantage of tab navigation.

All the following is for user on a Mac:

• Firefox: This MDN doc says that accessibility.tabfocus controls tab navigation, but if this is not set in Firefox (which it is not by default in Firefox), then the Mac setting in _System Preferences > Keyboard > Shortcuts > Use keyboard navigation to move_ is honored
• Safari: In my tests Safari _does not_ respect the System Preferences setting. Instead this feature must be activated within Safari: Preferences > Advanced > Accessibility

My question now is: what problem are we trying to solve?

1. Do we want to alter the markup to try to get around these Mac settings? To force tab navigation even though the user hasn't enabled these preference settings? My first gut feeling is this seems wrong, as we'd have to use non-semantic markup--and also, overriding user preferences seems kinda sketchy (even though here it is not for sketchy purposes).
2. Or do we want to educate the user that they must turn these settings on to get the best WordPress.com experience?

[ramonjd]

To force tab navigation even though the user hasn't enabled these preference settings?

I agree we should consider this. But given it's the "default" setting in Firefox, I wonder how many users would know (and care)?

Being able to tab through (what are ostensibly) navigable elements, I'd have thought, is a fairly basic feature of web browsing. Is there a good reason to have these disabled in the first place?

My first gut feeling is this seems wrong, as we'd have to use non-semantic markup

This might be "out there", but could we try forcing <Button /> to be a button element by assigning a click handler and getting rid of the href?

<Button /> used here in the sidebar:
https://github.com/Automattic/wp-calypso/blob/master/apps/full-site-editing/full-site-editing-plugin/wpcom-block-editor-nav-sidebar/src/components/nav-item/index.tsx#L44

Where <Button /> determines whether to use an <a /> or <button /> HTML element: https://github.com/WordPress/gutenberg/blob/master/packages/components/src/button/index.js#L67

Maybe there's a another way?

[autumnfjeld]

Ah, yes @ramonjd , <Button/> is a fine solution!! (And um ya I should have thought of that! 🤦‍♀️ )

I tested that on both Firefox & Safari (just using any ole website that has buttons & links to see what is tabable) and all good! Buttons & Inputs get hit by tabbing, <a /> links do not.

I will give that a go!!!! Thanks.

[andrewserong]

@autumnfjeld I think this one is quite a subtle issue. One part is the slightly odd default behaviour in Safari and Firefox on Mac of not tabbing to links, and I think we probably want to respect the default behaviour where it makes sense. That said, personally I think you should be able to at least tab to the back button in the sidebar.

But the issue that Phil describes (if I'm reading it correctly) is that in Firefox, if you start tabbing in the sidebar and keep on pressing the tab key, it'll actually end up tabbing through some of the elements in the right hand sidebar. But the tabbing should be constrained to _just_ the sidebar without escaping that area.

Here's a gif where you can see that eventually the buttons at the top of the UI are tabbed to, but because the sidebar should act like a modal, this shouldn't be allowed:

[andrewserong]

But yes, +1 to @ramonjd's idea of using a click handler to force the <Button/> component to render a button element!

[autumnfjeld]

Ah, sorry guys! I forgot one bit of info that I think speaks to your point @andrewserong

When you turn the accessibility settings on for Firefox, the the tabbing works fine in Firefox, at least in my own tests. Thus I assume if we changed the <a /> to <Button /> we'll get the desired & correct functionality.

[andrewserong]

Oh, @autumnfjeld one thing I forgot about when working on the nav sidebar locally, is that the feature is guarded behind both a filter and a config setting check (setting either will enable the feature) here: https://github.com/Automattic/wp-calypso/blob/master/apps/full-site-editing/full-site-editing-plugin/full-site-editing-plugin.php#L307

The way I've switched this feature on for local development is to set the WPCOM_BLOCK_EDITOR_SIDEBAR setting in wp-env by adding a .wp-env.override.json file to /apps/full-site-editing/.wp-env.override.json (this directory will be moved soon to /apps/editing-toolkit`).

The contents of my .wp-env.override.json file is:

{
    "config": {
        "WPCOM_BLOCK_EDITOR_SIDEBAR": true
    }
}

Then, running wp-env start again, the sidebar is switched on for me. (More info on this override JSON file at: https://github.com/WordPress/gutenberg/tree/master/packages/env#wp-envoverridejson)

[ramonjd]

at the moment I'm still leaning toward educating the user on how to turn on tab navigation in Safari & Firefox, rather than changing our markup to get around a setting the user simply needs to set.

Replying to your comment from https://github.com/Automattic/wp-calypso/issues/45240#issuecomment-681650775

I think I'm starting to agree.

The "Mac way" of enabling tab key navigation for links affects every site on the web, not just our sidebar.

This answer phrases it well: https://stackoverflow.com/a/49781088

I don't think that there is problem that you need to fix here.

Apple handles accessibility a little differently, and there are user preferences that must be enabled to allow for tabbing between links. This behavior is well known in the accessibility community, and I would not recommend implementing any custom solutions to modify this behavior, as it will likely only create other problems.

This article explains in more detail: http://www.weba11y.com/blog/2014/07/07/keyboard-navigation-in-mac-browsers/

cc @enriquesanchez who might have some ideas on how and whether we should tackle this

[enriquesanchez]

Hi everyone!

Folks who rely on keyboard navigation and/or assistive technology usually set their browsers and OS to work the way they need/prefer. We shouldn't change the sidebar's behavior or semantics to force what we think should be the right keyboard focus functionality.

I also think we should avoid educating the user about this. The complexity needed to _reliably_ detect the browser and OS would be too much, and then we wouldn't be able to know with certainty what the user's particular needs are.

As for semantics, we should avoid changing the behavior of an a element to be that of a button. These elements already carry the semantics needed by assistive technology to be interpreted and operated as such, and changing that will cause problems. Let links be links and buttons be buttons.

Related to constraining tabbing to the sidebar, we should definitively do this. Losing focus can be very disorienting. But there should also be an obvious way to close it and get back to the element that toggled it.

Last time I checked the sidebar had the role="dialog", I recommend taking a look at the WAI-ARIA Authoring Practices for this component. There's some really good info and guidelines on how to handle keyboard focus:

Like non-modal dialogs, modal dialogs contain their tab sequence. That is, Tab and Shift + Tab do not move focus outside the dialog. However, unlike most non-modal dialogs, modal dialogs do not provide means for moving keyboard focus outside the dialog window without closing the dialog.

Please feel free to reach out if I can help with anything else.

[p-jackson]

I agree that we're better leaving the semantics as they are i.e. using <a> for the links instead of <button>. Users will have other features like "announce the links that are available" and we don't want to break that feature in order to fix this. I also think not trying to educate the user is also the correct, they will have already set things up according to their preferences.

IMO the issue is that the withConstrainedTabbing HoC has a bug. It should work regardless of what the system/browser settings are (i.e. if that means it skips over all the links in the dialog then that's fine). The bug hasn't been exposed before because the other places it's used just so happen not to include links. We could open an issue in the GB repo but I can imagine it'd be pretty low priority given there's no user facing bug in GB.

I recommend taking a look at the WAI-ARIA Authoring Practices for this component.

Interestingly the approach taken in this example is totally different from the one taken by withConstrainedTabbing. It uses a couple of hidden focusable divs to mark the beginning and end of the dialog, instead of making assumptions about which elements are tabbable (which is what GB does).

[autumnfjeld]

Interestingly the approach taken in this example is totally different from the one taken by withConstrainedTabbing. It uses a couple of hidden focusable divs to mark the beginning and end of the dialog,

@p-jackson Could you clarify what example you are referring to?
Are you talking about a code example in the WAI-ARIA doc?

[p-jackson]

@autumnfjeld This example here: https://www.w3.org/TR/wai-aria-practices/examples/dialog-modal/dialog.html
You can click the "Add Delivery Address" button to see an example modal.

The HTML source is on that same page, and the JS source is here.

Took me a while to decipher how it works, but it doesn't look like it makes assumptions about what sort of elements are focusable (unlike withConstrainedTabbing). Instead if focus ever leaves the dialog then it tries to focus the first (or last) control in the dialog by iterating through every element until it finds an element that it succeeds on.

[ramonjd]

IMO the issue is that the withConstrainedTabbing HoC has a bug. It should work regardless of what the system/browser settings are (i.e. if that means it skips over all the links in the dialog then that's fine). The bug hasn't been exposed before because the other places it's used just so happen not to include links. We could open an issue in the GB repo but I can imagine it'd be pretty low priority given there's no user facing bug in GB.

What are your thoughts here?

Can we close this for now, or could we report our findings about withConstrainedTabbing over at Gutenberg?

[p-jackson]

I don't think we should close. It still feels like a serious bug imo. We should report in Gutenberg, but it'd be a bug in the component library, not with anything the user can reproduce in vanilla GB so don't think it'll get much attention.