Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safari doesn't tab to Prism code block #2695

Closed
dsenneff opened this issue Jan 5, 2021 · 19 comments · Fixed by #2799
Closed

Safari doesn't tab to Prism code block #2695

dsenneff opened this issue Jan 5, 2021 · 19 comments · Fixed by #2799

Comments

@dsenneff
Copy link

dsenneff commented Jan 5, 2021

When using the Safari desktop browser (v14) with a keyboard, Prism blocks don't appear in the tab order. So a keyboard-only user won't be able to tab to the block of code and scroll to view any overflowed content. Chrome, FireFox, and Edge work fine, I'm only seeing this in Safari.

I noticed this while testing a local site, but then checked the examples on Prism's site and I'm having the same issue, so it should be easy to replicate. No plugins or anything of that nature.

To replicate:

  1. Go to https://prismjs.com/index.html in Safari
  2. Navigate the page by using the tab key
  3. The focus indicator moves right past any Prism blocks of code. In this case, I'm able to tab to the "Examples" heading, then the next item in the focus order is the "view more example" link below the first scrollable block of code.
@mAAdhaTTah
Copy link
Member

Safari weirdly decided it doesn't tab-focus everything by default. You can enable it to do so if you go into Settings -> Advanced -> check "Press tab to highlight each item on a webpage".
Screen Shot 2021-01-05 at 7 42 24 AM

Beyond that, we're not doing anything specific to make it tabbable in those browsers, so I'm not sure this is something we can solve.

@dsenneff
Copy link
Author

dsenneff commented Jan 5, 2021

@mAAdhaTTah I'm aware of that setting and that's not the issue here, as I have that enabled already. And any regular keyboard user would have that enabled as well.

While you're not adding anything to make scrollable areas tabbable by default in Chrome and FireFox, by default areas do not overflow and instead grow to the size of their contents. If Prism is creating a structure and applying styles that create scrollable areas, then I feel it should take on a responsibility to make a good faith effort to make that content accessible. I don't think the fact that scrollable areas are tabbable by default in other browsers lets us off the hook.

I haven't tested fully, but I think a quick solution might be to just add a tabindex="0" to all of the pre elements created by Prism.

@mAAdhaTTah mAAdhaTTah reopened this Jan 5, 2021
@mAAdhaTTah
Copy link
Member

That's at least a reasonable principle, so let me look into it.

@RunDevelopment
Copy link
Member

Some additional info:

Chrome, Firefox, and Edge work fine.

I tested it on my machine and Firefox lets me tab to scrollable code blocks. Chrome and (Chromium) Edge have the problem you described. IE11 also has this problem.

@dsenneff
Copy link
Author

dsenneff commented Jan 8, 2021

Interesting, @RunDevelopment. Are you on Windows? I haven't tested on Windows, but on my Mac I don't experience the issue in Chrome or Chromium Edge. Might be the OSs handling key events different for UI elements, not sure.

@RunDevelopment
Copy link
Member

Yes, I am using Windows.

@Veyfeyken
Copy link

I'm on macOS.
Codeblock with scrollable content don't get focused in Chrome or Safari. They do get focus in Firefox.

This issue also get's flagged by Axe-core:
https://dequeuniversity.com/rules/axe/4.1/scrollable-region-focusable?application=AxeChrome

  • Adding tabindex="0" fixes the issue and provides consistency across browsers.
  • Developers won't have to add the attribute themselves afterwards to comply with WCAG Success Criterion 2.1.1 Keyboard
  • Keyboard users will be able to scroll and read all code blocks

@rschristian
Copy link

I don't think this is correct for accessibility. Focusable elements should have interactive semantics, but there's nothing about a block of code that inherently is interactive.

I've had to modify prism as I started to fail accessibility tests because of this incorrect behaviour.

If someone wants to be able to navigate code blocks with the keyboard then this is something they should add in themselves, as this breaks WCAG compliance.

@RunDevelopment
Copy link
Member

there's nothing about a block of code that inherently is interactive.

I disagree. Code blocks often have scroll bars, be it horizontal or vertical. These scroll bars must be interacted with in order to see the full content of a code block. However, in some browsers, this interaction is only possible with the tabindex attribute. As such, code blocks must be focusable for cross-browser accessibility.

If we made code blocks non-focusable again, we would make it impossible for keyword-users to see the full contents of some code blocks by default.

@Veyfeyken
Copy link

Related:
https://www.tpgi.com/short-note-on-improving-usability-of-scrollable-regions/

If someone wants to be able to navigate code blocks with the keyboard then this is something they should add in themselves, as this breaks WCAG compliance.

I assume you're getting a failure on Success Criterion 4.1.2 Name, Role, Value (A). This doesn't fail Success Criterion 2.1.1 Keyboard, it actually fixes it. Adding role="region" in combination with the tabindex="0" will handle the Name, Role, Value error.

@RunDevelopment
Copy link
Member

According to MDN, landmark roles should be used sparingly. Wouldn't role="generic" be more fitting?

@Veyfeyken
Copy link

Yes but role="generic" still gets flagged as an error on 4.1.2 Name, Role, Value unfortunately. Role="region" does not.

@Veyfeyken
Copy link

As role="region" creates an unnamed landmark, it would be good to identify it with an aria-label. Fox example:
<pre tabindex="0" role="region" aria-label="codeblock">
The above will receive focus so it can be scrolled with the keyboard. Screenreaders will announce a "region codeblock".

@rschristian
Copy link

I disagree. Code blocks often have scroll bars, be it horizontal or vertical. These scroll bars must be interacted with in order to see the full content of a code block. However, in some browsers, this interaction is only possible with the tabindex attribute.

You're confusing the fact that they can have scroll bars with an assumption that they will. This is not always the case.

Adding tabindex blindly breaks accessibility as, again, non-interactive areas should not be focusable. There are no actions a user can take.

This should be something users add for themselves unless you want to start detecting overflows and adding the attribute only when scroll bars actually appear.

If we made code blocks non-focusable again, we would make it impossible for keyword-users to see the full contents of some code blocks by default.

Right now you're making it more difficult for every keyboard user to navigate a site in an incorrect assumption.

At the end of the day, blocks of code are not inherently interactive. You cannot be add tabindex to them blindly.

I assume you're getting a failure on Success Criterion 4.1.2 Name, Role, Value (A)

No.

This doesn't fail Success Criterion 2.1.1 Keyboard, it actually fixes it.

If the area is non-interactive, adding tabindex fails this test.

@dsenneff
Copy link
Author

dsenneff commented Jan 24, 2022

I don't think it's fair to say that allowing all code blocks to be focusable "blindy breaks" the functionality for a keyboard user. Yes, in cases where the content doesn't overflow it will cause an extra tab stop, but that doesn't mean the page is broken for those users.

The spirit of WCAG SC 2.1.1 is that all content CAN be accessed with a keyboard, that doesn't mean that a user necessarily will interact with it. By being consistent and predictable, users will learn that all code blocks are accessible even though a particular one might not require it. To my knowledge, a code block that is focusable but doesn't require scrolling to view the content is not a scenario that fails this SC.

Content can change: authors can make edits that now cause a code block to overflow. And users can change their viewing specifics: they can revisit the page on a different device or different size browser window, they might enlarge their text to view something better, or other actions that create overflow where it didn't exist before. So a consistent experience is key.

Developers can also mitigate confusion with proper headings and other information paired with the code block, such as "CSS Example 3 (content may overflow)".

@extra808
Copy link

There are pros and cons but I think what PrismJS is now doing, adding tabindex=0 with no role or name, is best for users. Including tabindex removes a barrier for keyboard (and keyboard-equivalent) users to enable access to scrollable codeblocks. Since the tabindex is always present, it does add an unnecessary tab stop to non-scrollable codeblocks for keyboard users but that only makes page navigation a little worse, the trade-off is worth it. If there was a whole set of codeblocks that might not be of interest to all readers, the author could precede the set with a "skip link" or put them all within a disclosure widget (like details-summary elements). In theory, you could have JavaScript checking whether each codeblock is scrollable to add/remove the tabindex attribute (or toggle the value between 0 and -1) but that doesn't sound like a worthwhile trade-off (to develop or to have continually running on every page).

Regarding MDN's WCAG Keyboard page, it's not an authoritative source but it is useful. It says "focusable elements should have interactive semantics." A scrollable codeblock is interactive, you can scroll it, but there are no interactive semantics that convey "scrollability." role=region and an accessible name, like aria-label="code block" could be added but they would be no benefit for those who need the tab stop and would make the user experience worse for others. Screen reader users don't need the tab stop, the screen reader gives them access to all the content without it. The role and accessible name will only be conveyed to the screen reader user but they don't need it; they're unlikely to use the regions for landmark navigation and if all the codeblocks have the same accessible name, there's nothing to differentiate them.

The MDN page says "focusable elements should have interactive semantics" but for other subjects, it uses the word "must," like "interactive elements must be focusable." "Must" is a requirement, "should" is a recommendation or a description of typical situations; this case is an exception, for the reasons I've given.

Additionally, in the browsers that are automatically adding a tab stop for scrollable elements, they are not adding any semantics with it; two wrongs don't make a right but they do serve as real-world examples of focusable elements without interactive semantics.

If tests are failing on elements solely because they have tabindex=0, the tests should be improved. Adding roles and names to satisfy the tests would benefit only developers and make the user experience worse for some people.

@oldrup
Copy link

oldrup commented Apr 14, 2022

I was confused about the new tabindex of the pre element to be honest, as when testing keyboard navigation on my site "something" was getting focused, but I didn't know what. So I'm experimentally adding a focus outline to pre elements with tabindex like

/** Improve general outline visibility, especially in firefox **/ :is(a, button, pre[tabindex="0"]):focus { outline-width: var(--outline-width); outline-style: var(--outline-style); outline-color: var(--linkHoverColor); outline-offset: var(--outline-offset); }

I think adding an aria-label to the element would benefit users of screen readers, but that should probably be done manually? Has anyone been researching this, maybe done some accessibility testing?

@RunDevelopment
Copy link
Member

"something" was getting focused, but I didn't know what

Firefox does have a subtle focus outline, so unless you overwrite that, it should be clear which element is focused.

image

I think adding an aria-label to the element

I don't think that we can do that on a library level. The main problem is that screen readers seem to just read out the value of the label. So the screen reader saying "code block" won't help non-English speaking people.

@oldrup
Copy link

oldrup commented Apr 17, 2022

Thank you @RunDevelopment for your insights. Those are valid points. I fixed my firefox styling to make the focus outline properly, and you're right, an aria label should be descriptive and thus probably added manually - a generic "code block" label is not helping much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants