Opened 3 years ago

Closed 3 years ago

#14931 closed Bug (fixed)

AVT: Title for all buttons in CK Editor Toolbar not displayed for Keyboard only User

Reported by: Satya Minnekanti Owned by: Marek Lewandowski
Priority: Normal Milestone:
Component: Accessibility Version:
Keywords: IBM Cc: Irina

Description

Steps to reproduce

  1. Open nightly build.
  2. Navigate to toolbar by pressing Alt + F10
  3. Navigate to each toolbar button using TAB key (keyboard only user).

Issue: Label for each button(ex: Source) not displayed for Keybaord only user. This is violation of AVT Checkpoint Checkpoint 2.1.1 Keyboard http://www-03.ibm.com/able/guidelines/ci162/keyboard.html

Attachments (7)

Tooltips - title attribute on its own is not a compliant solution.html.hml (2.8 KB) - added by Satya Minnekanti 3 years ago.
Tooltips - Title attribute on its own is not a compliant solution.html (2.8 KB) - added by Satya Minnekanti 3 years ago.
Screenshot from 2016-10-14 15-36-07.png (71.1 KB) - added by Tade0 3 years ago.
Destroyed layot
Screenshot from 2016-10-14 15-42-34.png (76.8 KB) - added by Tade0 3 years ago.
Moved tooltip
fixed-tooltip.png (11.6 KB) - added by Tade0 3 years ago.
Screen Shot 2016-11-10 at 17.25.15.png (22.2 KB) - added by kkrzton 3 years ago.
Screen Shot 2016-11-21 at 14.24.27.png (17.5 KB) - added by kkrzton 3 years ago.

Download all attachments as: .zip

Change History (36)

comment:1 in reply to:  description Changed 3 years ago by Satya Minnekanti

Replying to satya:

Steps to reproduce

  1. Open nightly build.
  2. Navigate to toolbar by pressing Alt + F10
  3. Navigate to each toolbar button using TAB key (keyboard only user).

Issue: Label for each button(ex: Source) not displayed for Keybaord only user. This is violation of AVT Checkpoint Checkpoint 2.1.1 Keyboard http://www-03.ibm.com/able/guidelines/ci162/keyboard.html

Please look at this page http://pokgsa.ibm.com/gsa/pokgsa/projects/s/swf_design/design/Team-Folders/Randy-B/accessibility/aqg/tooltips.html for examples on how to fix this issue

comment:2 Changed 3 years ago by Marek Lewandowski

Unfortunately we can't resolve http://pokgsa.ibm.com/gsa/pokgsa/projects/s/swf_design/design/Team-Folders/Randy-B/accessibility/aqg/tooltips.html host, it does look like a private one.

We're investigating the issue.

comment:3 Changed 3 years ago by Marek Lewandowski

Status: newconfirmed

The ticket is valid, button tooltips would be a good a11y practice. Implementation would require some work and would put a notable footprint on the editor size so for sure it has to be contained within a plugin.

comment:4 Changed 3 years ago by Tade0

Owner: set to Tade0
Status: confirmedassigned

comment:5 Changed 3 years ago by Tade0

Status: assignedreview

Added a "Keyboard Tooltip" plugin that adds a tooltip when a button has focus.

Changes pushed to branch:t/14931.

Last edited 3 years ago by Tade0 (previous) (diff)

comment:6 Changed 3 years ago by Marek Lewandowski

Status: reviewreview_failed
  1. Please put the styles inline, there's no reason to make another HTTP request just to get such a tiny file.
  2. This CSS should be added in a onLoad method instead of init. The reason for it is that it will get executed multiple times for each editor, while it's enough to do it just once.

comment:7 Changed 3 years ago by Tade0

Status: review_failedreview

Corrected the errors.

Tested in IE9, IE11, Chrome, Firefox.

comment:8 Changed 3 years ago by Tomasz Jakut

Status: reviewreview_failed

There is one issue with the current solution: tooltips for the bottom line of buttons are cropped. In Chrome it's just unaesthetic, but in Firefox the issue is bigger, e.g. "g" looks like "o". Therefore tooltips are very hard to read and can be misleading.

The problem is caused by the fact that the container for all toolbars is floated and overflow: hidden is used to preserve its dimensions. However it also cause the cropping of tooltips. Perhaps the overflow: hidden could be replaced with clearfix hack by the plugin to properly display tooltips at the bottom?

Changed 3 years ago by Tade0

Destroyed layot

Changed 3 years ago by Tade0

Moved tooltip

comment:9 Changed 3 years ago by Tade0

I tried that, but it broke the layout.

Then again moving the tooltip a little bit to the top and to the right for good measure seems to look better. What do you think?

comment:10 Changed 3 years ago by Tomasz Jakut

Yep, the option with moved tooltip looks very good.

comment:11 Changed 3 years ago by Tade0

Status: review_failedreview

Ok, so I'll be going with it.

Changes pushed to branch:t/14931.

comment:12 Changed 3 years ago by Tomasz Jakut

Status: reviewreview_passed

LGTM.

comment:13 Changed 3 years ago by Marek Lewandowski

Updated the tooltip - removed the opacity / because it lowers the accessibility (low contrast). Also moved the tooltip a little down.

comment:14 Changed 3 years ago by Tade0

Replaced the native tooltip with this one, added a 1px white border for better visibility in high contrast mode, solved truncation problem.

Changes pushed to branch:t/14931.

comment:15 Changed 3 years ago by Marek Lewandowski

The only thing I'm missing here is support for high contrast, and in fact we should not show tooltip in high contrast. The reason why is that in high contrast labels are visible so there's no need to duplicate this information and make UI more complex.

comment:16 Changed 3 years ago by Marek Lewandowski

Status: review_passedreview_failed

I've added a manual test for colorbutton integration. With title attribute removed from colors, those would not have any accessible label for screen readers. I have updated the existing logic to add aria-label for these cases. However still R fails because:

  • label is not shown for "automatic" option nor for "more colors", see screen: https://i.imgur.com/w7qqfgG.png

comment:17 Changed 3 years ago by Marek Lewandowski

Related issue #16373.

Changed 3 years ago by Tade0

Attachment: fixed-tooltip.png added

comment:18 Changed 3 years ago by Tade0

Status: review_failedreview

Fixed:

https://dev.ckeditor.com/raw-attachment/ticket/14931/fixed-tooltip.png

Works for moono-lisa as well.

comment:19 Changed 3 years ago by Marek Lewandowski

Status: reviewreview_passed

LGTM, before closing the ticket, please publish plugin on a separate repository cksource/ckeditor-plugin-keyboardtooltip.

comment:20 Changed 3 years ago by kkrzton

We have discovered one issue. As a tooltip is applied as :before element with relative positioning to the toolbar button (a) it works fine as long as all buttons have position: relative. That is the case for moono-lisa skin and normal buttons in moono skin. However, for dropdowns in moono skin, they have position: static so the tooltip is not rendered in a correct position (see attached screenshot).

If we are planning to deliver a generic plugin which will work with different skins I think it might be hard to achieve with the current approach. Probably we should consider global positioning of tooltips instead of positioning relatively to the buttons.

Changed 3 years ago by kkrzton

comment:21 Changed 3 years ago by kkrzton

Status: review_passedreview_failed

comment:22 Changed 3 years ago by kkrzton

Owner: changed from Tade0 to kkrzton
Status: review_failedassigned

comment:23 Changed 3 years ago by Marek Lewandowski

Owner: changed from kkrzton to Marek Lewandowski

comment:24 Changed 3 years ago by Marek Lewandowski

Status: assignedreview

Pushed branch:t/14931 to review.

I've fixed a number of things, like:

  • compatibility with inline editor (it was formerly broken)
  • tooltips are no longer cropped within dropdown - element is now placed in the outer document
  • plugin will no longer throw an exception if only one of color buttons is added to the toolbar

There's a know issue that the tooltip is not aligned correctly only for the first time you open color button palette using the keyboard. Any subsequent calls are fine.

comment:25 Changed 3 years ago by kkrzton

Status: reviewreview_failed

I can see two issues:

  1. The tooltips are not positioned correctly for moono-lisa skin (checked on Chrome and Firefox - see screenshot).
  2. If you resize the viewport so that toolbar buttons positions changes the tooltip stays at it initial (and now invalid) position. This is the very rare edge case so I am not sure if it's worth fixing.

Overall, global positioning of tooltips is probably a way to go. Maybe you may consider using getBoundingClientRect instead of offsets as it might be more accurate?

Changed 3 years ago by kkrzton

comment:26 in reply to:  25 Changed 3 years ago by Marek Lewandowski

Status: review_failedreview

Replying to k.krzton:

I can see two issues:

  1. The tooltips are not positioned correctly for moono-lisa skin (checked on Chrome and Firefox - see screenshot).

That's right, the problem is fixed by replacing [CKEDITOR.addCss with document.appendStyleText](https://github.com/cksource/ckeditor-dev/commit/a3a9112039e519e214724cbb5ca39c46e1f9ca84). Also I've added a manual test for Kama skin.

  1. If you resize the viewport so that toolbar buttons positions changes the tooltip stays at it initial (and now invalid) position. This is the very rare edge case so I am not sure if it's worth fixing.

Yes it won't reposition itself on resize event, but it will do so on next button focus. We'll extract this into a separate issue.

Overall, global positioning of tooltips is probably a way to go. Maybe you may consider using getBoundingClientRect instead of offsets as it might be more accurate?

I was considering bounding rect, but it proved to be wrong in some cases, e.g. on our manual test pages. This way of computing offsets had correct results in this cases.

comment:27 Changed 3 years ago by kkrzton

Status: reviewreview_passed

LGTM!

comment:28 Changed 3 years ago by Marek Lewandowski

For the record - the tooltips are easily stylable, you could simply add following rule to your stylesheet:

div.cke_tooltip.cke_reset.cke_reset_all{ background: white; color: black; border: solid orange 1px; border-radius: 5px;}

And this will change the appearance of the tooltips.

comment:29 Changed 3 years ago by Marek Lewandowski

Resolution: fixed
Status: review_passedclosed

This feature will be merged to major in issue #16373.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy