Problem/Motivation

In elements.css, all links are styled to have no outline on focus. However, there are places where links used as buttons, and those should not have their focus outline overridden. They function as buttons and are not appearing within other text, so they should meet Focus Visible criteria.

Steps to reproduce

Go to admin/config/content/formats/manage/basic_html with CKEditor set as the text editor.
In "Toolbar configuration", the buttons do not get an outline when focused.

Proposed resolution

add a :not([role="button"]) to the selectors of the style removing outlines from focused links.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#12 Screen Shot 2023-02-09 at 9.36.20 PM.png40.19 KBbnjmnm
#12 Screen Shot 2023-02-09 at 9.38.14 PM.png104.37 KBbnjmnm
#7 Screen Shot 2021-05-25 at 19.51.00.png8.3 KBlauriii

Issue fork drupal-3208088

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bnjmnm created an issue. See original summary.

bnjmnm opened merge request !529

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Active » Needs review

Here is a video of keyboard-navigating the CKEditor buttons, they get a focus outline!
https://youtu.be/cfudLyYMkTQ

zrpnr’s picture

zrpnr
he/him
Primary language English
Location UTC-7
commented
Status: Needs review » Reviewed & tested by the community

Can confirm that the css classes added in #2 are a big improvement, focus was invisible on all CKE toolbar buttons in Seven.
Changing to :not([role="button"]) is a non-destructive and careful way to fix this problem!

I noticed that the focus for these buttons was missing in Claro, but it's due to a conflict with box-shadow in ckeditor.admin.css, and is unrelated to this issue.
Created #3208474: CKE admin toolbar - improve visibility on focus

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3206687: Toolbar UI accessibility review, +#3208474: CKE admin toolbar - improve visibility on focus

Love that a bug discovered while working on CKEditor 5 (#3206687: Toolbar UI accessibility review) led to a bug being discovered in CKEditor 4 😄

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new Screen Shot 2021-05-25 at 19.51.00.png8.3 KB

The MR increases selector specificity and there is a regression with the table headings sort links as a result of that. Focused table headings have now duplicate underline:

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Component: Seven theme » Claro theme

Seven is no longer in core. I asked in #bugsmash where this issue should move to. Lendude responded saying that the same code is in Claro. Therefore I am moving this to Claro and adding lendude to the triage meta, #3332483: [Meta] Seven issue triage

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Closed (outdated)
Issue tags: -Accessibility
StatusFileSize
new Screen Shot 2023-02-09 at 9.38.14 PM.png104.37 KB
new Screen Shot 2023-02-09 at 9.36.20 PM.png40.19 KB

While it's true that the code I mentioned was a problem is still in Claro, that same CSS file (in Claro) includes focus styling of equal-to-greater specificity that appears after the outline-removing styles, so the focus ring shows up for all links (and pretty much anything receiving focus).

.page-wrapper *:focus, .ui-dialog *:focus {
    outline: var(--focus-outline);
    box-shadow: var(--focus-box-shadow);
}