Opened 5 years ago

Closed 4 years ago

#11387 closed Bug (fixed)

role="radiogroup" should be applied only to radio inputs' container.

Reported by: Irina Owned by: Marek Lewandowski
Priority: Normal Milestone: CKEditor 4.4.2
Component: Accessibility Version: 4.0
Keywords: IBM Cc: Teresa Monahan, Satya Minnekanti, peter

Description

To Reproduce:

  1. Open any CKEditor sample in FF.
  2. Open any dialog that has input fields, e.g. insert table dialog
  3. Inspect input fields ( e.g. rows) using Firebug

Problem: Each inputs' container has a role="radiogroup"

Elements that use 'role' must contain required child elements for the role to enable compatibility with assistive technologies, e.g.

<div role="radiogroup">
    <div role="radio">radio element markup</div>
</div>

Change History (8)

comment:1 Changed 5 years ago by Jakub Ś

Component: GeneralAccessibility
Status: newconfirmed
Version: 4.3.14.0

Looking at below links:

I agree that this role should not be used on inputs as it doesn't fit to WAI definition at all.

comment:2 Changed 4 years ago by Marek Lewandowski

Owner: set to Marek Lewandowski
Status: confirmedreview

That's correct, attr [role="radiogroup"] should not be used here. This layer is used simply as a wrapper. I've also noticed that it reuses the same label as in contained input, which also is invalid.

As a result I changed the role to presentation, and removed redundant label.

Pushed to t/11387 at dev.

Last edited 4 years ago by Marek Lewandowski (previous) (diff)

comment:3 Changed 4 years ago by Piotrek Koszuliński

Status: reviewreview_failed

Well, we should first check why the attribute was added there. I agree it looks odd there, but there might be a reason. Blame leads me to #10866. Please verify it.

comment:4 Changed 4 years ago by Marek Lewandowski

Status: review_failedreview

I've talked with Fred and change had solely semantic purpose, since it doesn't affect JAWS. I've added the option to specify role in elementDefinition passed to CKEDITOR.ui.dialog.labeledElement, same applies to label. Both options are documented.

Pushed to t/11387 at dev and t/11387 at tests.

comment:5 Changed 4 years ago by Piotrek Koszuliński

I rebased branch:t/11387 and ported tests to it.

comment:6 Changed 4 years ago by Olek Nowodziński

Status: reviewreview_passed

I pushed a commit https://github.com/cksource/ckeditor-dev/commit/2e2e9484bf97332c32f93071bef332f14679873e that replaces RegExp-based assertions with DOM-based ones, which are much safer and straightforward.

comment:7 Changed 4 years ago by Olek Nowodziński

Milestone: CKEditor 4.4.2

comment:8 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on master with git:2974078.

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