Opened 6 years ago

Closed 6 years ago

#7873 closed Bug (fixed)

Disable uiElement API has changed - no longer works as expected

Reported by: tmonahan Owned by:
Priority: Normal Milestone: CKEditor 3.6.1
Component: UI : Dialogs Version: 3.6.1
Keywords: IBM Cc: damo, satya, jamescun


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 follow-up: Changed 6 years ago by tmonahan

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 fredck

  • Component changed from General to UI : Dialogs
  • Milestone set to CKEditor 3.6.1
  • Status changed from new to confirmed

Replying to tmonahan:

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

comment:3 Changed 6 years ago by garry.yao

  • Resolution set to fixed
  • Status changed from confirmed to closed

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