Opened 11 years ago

Closed 11 years ago

#3104 closed Bug (fixed)

Tab focus on editor with tabindex after the editor has been focused is wrong

Reported by: Martin Kou Owned by: Frederico Caldeira Knabben
Priority: Must have (possibly next milestone) Milestone: CKEditor 3.0
Component: Accessibility Version: SVN (FCKeditor) - Retired
Keywords: Oracle Review+ Cc: Senthil

Description

Steps to reproduce:

  1. Add 6 text areas in sample.html around the script tag with document.write( CKEDITOR.samples.htmlData ); - 3 text areas before it and 3 text areas after it.
  2. Added tabindex attributes to the text areas as 1,2,3,5,6,7 respectively.
  3. Add tabindex="4" to the textarea in replacebyclass.html.
  4. Open replacebyclass.html in Firefox or IE.
  5. Select one of the text areas before the editor, press tab until the editor is focused.
  6. Press tab again.
  7. The text boxes after the editor are skipped.

The skipping problem occurs also for Shift-Tab.

Attachments (4)

3104.patch (3.0 KB) - added by Martin Kou 11 years ago.
3104_2.patch (6.3 KB) - added by Martin Kou 11 years ago.
3104_3.patch (5.0 KB) - added by Martin Kou 11 years ago.
3104_4.patch (15.1 KB) - added by Frederico Caldeira Knabben 11 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 11 years ago by Martin Kou

It seems there's no way of making tab index work normally with iframes - the browsers are treating the tab key press from inside the iframe inconsistently within themselves. It seems the only real way to fix it is that the developer tells CKEditor what element to put focus on if his user presses Tab or Shift-Tab within CKEditor.

Changed 11 years ago by Martin Kou

Attachment: 3104.patch added

comment:2 Changed 11 years ago by Martin Kou

Keywords: Review? added

comment:3 Changed 11 years ago by Martin Kou

Keywords: Review- added; Review? removed

The solution is found to be unacceptable after talking with Senthil - they don't always know in advance what elements would come before and after the editing area. A different approach is needed.

Changed 11 years ago by Martin Kou

Attachment: 3104_2.patch added

comment:4 Changed 11 years ago by Martin Kou

Keywords: Review? added; Review- removed

Turns out the logic implemented in #3098 was headed for the wrong direction.

The bug we're having now is caused by the tab focus logic implemented in the tab plugin - so it's not a browser bug as I've previously suspected. The tab plugin is actually handling ta and shift-tab events from within the editing area but it's not well integrated with the tab index fix in #3098 and so it didn't work. So the better way of handling this is - instead of adding hacks into wysiwygarea and sourcearea plugins, we should add the missing integration logic between the tabindex fix in #3098 and the tab plugin.

comment:5 Changed 11 years ago by Martin Kou

Keywords: Review- added; Review? removed

Ouch, it's not working for IE.

Changed 11 years ago by Martin Kou

Attachment: 3104_3.patch added

comment:6 Changed 11 years ago by Martin Kou

Keywords: Review? added; Review- removed

Ok, it's found to be a small bug in the tab plugin - domElement.setAttribute('tabindex', x) does not work in IE. It must be domElement.setAttribute('tabIndex', x);

comment:7 Changed 11 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
Owner: changed from Martin Kou to Frederico Caldeira Knabben
Status: newassigned
  • The custom key handled in the sourcearea plugin is not needed. You can use the editor.keytrokeHandler for that, which will give you the benefit of still having all other keys workings (like ALT+F10).
  • The IE fix for tabindex should go into setAttribute, not at the origin.
  • Base visibility check on styles only is not accurate, as it has no difference for children of hidden elements. There is a better way for that check.
  • There are still some cases where the TAB is not moving properly (replacebycode sample, for example).

I've also noted that the tab plugin needs some review. I'm taking over this ticket as I've already worked on it while reviewing, and for urgency of it.

Changed 11 years ago by Frederico Caldeira Knabben

Attachment: 3104_4.patch added

comment:8 Changed 11 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review- removed
  • This new patch also makes it possible to configure a tabindex for the editor, even if the textarea doesn't have it.
  • The tab plugin has been totally reviewed.
  • element::getPreviousSourceElement has been fixed as it has been ported from V2 wrongly.

comment:9 Changed 11 years ago by Martin Kou

Keywords: Review+ added; Review? removed

comment:10 Changed 11 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

Fixed with [3222].

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