Ticket #8985 (closed Bug: fixed)

Opened 2 years ago

Last modified 22 months ago

IE - Enter key not being properly processed by dialogs

Reported by: lynne_kues Owned by: fredck
Priority: Normal Milestone: CKEditor 3.6.4
Component: UI : Dialogs Version: 3.0
Keywords: IE9 IBM Cc: monahant@…

Description

Enter key is not being consumed by dialog even though the dialog has focus. Instead the keystroke is propagating up and is being processed by our application while the dialog is opened. See attachment for an explanation. config.dialog_startupFocusTab = true;

I looked at dialog.js to see if I could figure out what was going on. If I modify focusKeydownHandler to consume the enter key in all instances, things work as expected.

Attachments

table.png (104.3 KB) - added by lynne_kues 2 years ago.
replacebycode.html (3.7 KB) - added by j.swiderski 23 months ago.
8985.patch (3.9 KB) - added by garry.yao 22 months ago.
8985_2.patch (6.5 KB) - added by fredck 22 months ago.
8985_3.patch (6.9 KB) - added by fredck 22 months ago.
8985_4.patch (6.9 KB) - added by fredck 22 months ago.

Change History

Changed 2 years ago by lynne_kues

comment:1 Changed 2 years ago by j.swiderski

  • Status changed from new to pending

I have just checked this issue in latest CKEditor 3.6.3.

First enter puts focus in Rows field but second inserts the table. There is no saving or other undesired action. Were you able to reproduce this problem in plain CKEditor or only inside your application?

comment:2 Changed 2 years ago by tmonahan

  • Cc monahant@… added

comment:3 Changed 2 years ago by lynne_kues

I can only reproduce this inside our application and I do think it is related to running CKE inside dojo and/or our framework. If the CKE dialog has focus, why is the Enter key event being propagated, which it is and which is then being picked up by our application. As I indicated, if I change focusKeyDownHandler to consume the event (i.e., evt.stop(); evt.data.preventDefault();), our application doesn't process it, but that's not a fix and it's unclear as to why the event doesn't trigger OK like it does in FF. Any suggestions on how to further debug this? I am at a bit of a loss and unclear as to why our application would effect this dialog. We are not extending the table dialog.

Last edited 2 years ago by lynne_kues (previous) (diff)

comment:4 Changed 2 years ago by fredck

Do you think you're able to produce a reduced test case for this issue, so we can confirm the problem at our side? I think this is the only way to be able to give you some more details about it. Thanks!

comment:5 Changed 2 years ago by lynne_kues

The issue is that on IE, the first button it finds is being set as the default button. You'll notice in the screenshot that the Save button has a dark outline around it. Then when the table dialog is opened and the Enter key pressed, because the event is being propagated, the Save button is being invoked.

Changed 23 months ago by j.swiderski

comment:6 Changed 23 months ago by j.swiderski

  • Status changed from pending to confirmed
  • Version changed from 3.4.2 to 3.0

OK, I was able to reproduce this problem in IEs from CKEditor 3.0.

It seems that config.dialog_startupFocusTab = true; is not required and all this TC needs is submit button above CKEditor.

To reproduce:

  1. Put attached file in _samples folder and load it in a browser
  2. Open table dialog and press Enter once or twice - focus will go to submit button

@lynne_kues you've said that you have managed to overcome this problem. Could you perhaps provide a patch write where (path and line to file) and what have you changed.

Last edited 22 months ago by j.swiderski (previous) (diff)

comment:7 Changed 23 months ago by lynne_kues

In 3.6.2 code base, dialog/plugin.js, method focusKeyDownHandler, line 449, I added the following code

} else if (keystroke == 13) {

evt.data.preventDefault();

}

This resolves the issue; however, the fix is not quite correct. As a side effect of this change, if the Ok button has focus and the enter key is pressed, the click action is not fired.

comment:8 Changed 22 months ago by garry.yao

  • Status changed from confirmed to pending

I'm unable to reproduce it on IE with the attached sample.

comment:9 Changed 22 months ago by j.swiderski

@garry.yao I have modified the description, could you please test the attached sample once more?

You have to press Enter once or twice. With one of those "Enters" focus will go to submit button.

Last edited 22 months ago by j.swiderski (previous) (diff)

comment:10 Changed 22 months ago by j.swiderski

  • Status changed from pending to confirmed
  • Keywords IE9 added

Ok, I have checked this once more and problem is only reproducible in IE9.

Changed 22 months ago by garry.yao

comment:11 Changed 22 months ago by garry.yao

  • Status changed from confirmed to review
  • Owner set to garry.yao
  • Milestone set to CKEditor 3.6.4

comment:12 Changed 22 months ago by fredck

  • Status changed from review to review_failed

Unfortunately the idea of making it work for text fields only is wrong. The ENTER action is not limited to that type of field only. It should work in the same way when over a combo or a checkbox or whichever UI element that doesn't handle ENTER.

In poor words, whenever ENTER reaches the dialog holder element, the OK button must be activated and the event must stop propagating. It doesn't matter what kind of element is focused.

Changed 22 months ago by fredck

comment:13 Changed 22 months ago by fredck

  • Owner changed from garry.yao to fredck
  • Status changed from review_failed to review

The new patch is based on the previous one, but making the event catching through the entire dialog, regardless the focused element.

It helps also fixing a regression of #4269 with FF, which was still happening when focused on the dialog tabs. It looks like something changed on the way FF handles key events.

comment:14 Changed 22 months ago by fredck

  • Status changed from review to assigned

Changed 22 months ago by fredck

comment:15 Changed 22 months ago by fredck

  • Status changed from assigned to review

This new patch is very similar to the previous, but is smarter on figuring out if the ENTER key should be accepted by the focused element.

comment:16 Changed 22 months ago by garry.yao

  • Status changed from review to review_failed
  1. Open link dialog;
  2. Type something into the URL field;
  3. Click on "Protocol" select field to open drop-down;
  4. Press enter key
  • Actual: Dialog get closed;
  • Expected: Dropdown get closed and focus return to the select element.

Thea above applies also to the ESC key.

Changed 22 months ago by fredck

comment:17 Changed 22 months ago by fredck

  • Status changed from review_failed to review

So it looks like we have no alternative other than not enabling the ok button when ENTER on select elements.

Btw, I've noticed that ENTER has the "click" action on closed select boxes as well. This can be checked on the link dialog. Just select Link Type with the keyboard, without opening the select box, and hit ENTER. You'll see that ENTER changes the dialog accordingly. This helps justifying the proposed patch.

comment:18 Changed 22 months ago by garry.yao

  • Status changed from review to review_failed

Thea above applies also to the ESC key.

This comment is not taken into the new patch.

comment:19 Changed 22 months ago by garry.yao

  • Status changed from review_failed to review_passed

Ok, one justification is that it may looks strange if ESC doesn't work consistently across all fields... and cosnidering that we have that same behavior on v3, let's keep it the current way.

comment:20 Changed 22 months ago by fredck

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

Fixed with [7516].

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