Opened 6 years ago

Closed 6 years ago

#7873 closed Bug (fixed)

Disable uiElement API has changed - no longer works as expected

Reported by: Teresa Monahan Owned by:
Priority: Normal Milestone: CKEditor 3.6.1
Component: UI : Dialogs Version: 3.6.1
Keywords: IBM Cc: Damian, Satya Minnekanti, James Cunningham

Description

The APIs for disabling uiElements in plugins/dialog/plugin.js has changed. It previously disabled the input element. However it now sets the disabled attribute on the uiElement itself. Therefore even though a uiElement is disabled, it is still possible to enter text in it's associated input element.

An example of this can be seen on the table dialog.

  1. Create a table
  2. Right click on the table and select Table Properties from the context menu.
  3. Notice that the Rows and Columns fields look disabled.
  4. Place the cursor in either the Rows or Columns field and type some text.


Problem: You should not be able to type in these fields. The input elements should be disabled directly instead of disabling the parent div element.

Apart from allowing users to type into disabled fields, the 3.6.1 implementation of this also has an accessibility impact. Disabling the parent div element means that the child input element is not properly disabled and it is therefore still exposed as a focusable and editable field to accessibility APIs. Also disabled is not a supported attribute on div elements in the HTML specifications.

Change History (3)

comment:1 Changed 6 years ago by Teresa Monahan

Actually it looks like this new issue was introduced as a result of Ticket #6657. We would still like the behaviour described in ticket #6657 i.e that the cke_disabled CSS class gets added to the UI Element's div rather than the field's input element. However we believe the disabled attribute should be set directly on the input element as described in this ticket.

comment:2 in reply to:  1 Changed 6 years ago by Frederico Caldeira Knabben

Component: GeneralUI : Dialogs
Milestone: CKEditor 3.6.1
Status: newconfirmed

Replying to tmonahan:

Your comments describes it properly. The current way is definitely wrong.

comment:3 Changed 6 years ago by Garry Yao

Resolution: fixed
Status: confirmedclosed

Fixed with [6947], sorry for messing up the commit message.

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