Opened 3 years ago

Closed 22 months ago

Last modified 20 months ago

#14407 closed Bug (fixed)

Non-editable widgets can be edited in IE

Reported by: pollyfahey Owned by: Tade0
Priority: Normal Milestone: CKEditor 4.7.0
Component: General Version:
Keywords: Cc:

Description

Steps to reproduce

  1. Add custom table widget as attached.
  2. In editor add table.
  3. Click on the bottom border of the widget and type.

Expected result

The text should be added underneath the widget.

Actual result

The text is added in the widget.

Other details (browser, OS, CKEditor version, installed plugins)

Observed in IE 11. Using CKEditor 4.5.3. Windows 10.

Attachments (3)

plugin.js (888 bytes) - added by pollyfahey 3 years ago.
testtable.zip (1.4 KB) - added by Jakub Ś 3 years ago.
Attached corrected version of the plugin.
2016-02-15_1700.swf (644.8 KB) - added by Jakub Ś 3 years ago.
Screen cast showing the problem

Download all attachments as: .zip

Change History (20)

Changed 3 years ago by pollyfahey

Attachment: plugin.js added

Changed 3 years ago by Jakub Ś

Attachment: testtable.zip added

Attached corrected version of the plugin.

Changed 3 years ago by Jakub Ś

Attachment: 2016-02-15_1700.swf added

Screen cast showing the problem

comment:1 Changed 3 years ago by Tomasz Jakut

Milestone: CKEditor 4.6.0
Status: newconfirmed

comment:2 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.0CKEditor 4.6.1

comment:3 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.1CKEditor 4.7.0

We won't be able to contain it within 4.6.1 - moving to next major release.

comment:4 Changed 2 years ago by Tade0

Owner: set to Tade0
Status: confirmedassigned

comment:5 Changed 2 years ago by Tade0

I noticed that after creating that widget and adding the attribute contenteditable="false" to some of its elements it's still possible to edit them.

comment:6 Changed 2 years ago by Tade0

Status: assignedreview

Given that what is happening here is something, that should be impossible I went with a solution that covers this specific case.

Changes pushed to branch:t/14407.

comment:7 Changed 23 months ago by Tomasz Jakut

Status: reviewreview_failed

Test is not working at all – it seems that it tries to load nonexistent plugins. After removing these imports, there is no way to add widget into editable. Apart from that test could be moved into tests/plugins/widget.

After dealing with the test I noticed that I still could insert something into noneditable widget:

  1. Move into the widget using Up Arrow.
  2. Immediately after moving into it, press any character key.

The letter appears at the beginning of the widget.

comment:8 Changed 23 months ago by Tade0

Apparently in this case IE11 in Windows 7 behaves differently than IE11 in Windows 10.

comment:9 Changed 23 months ago by Tade0

There's one solution, but it isn't pretty:

For now, a selection check is done on keyup with a timeout(cited reason was performance, but that was in 2011, so it probably doesn't apply now). Adding the same check on keydown solves this particular problem, but breaks several tests.

Currently trying to determine why does it do that and is it significant enough to justify not going with this solution.

comment:10 Changed 23 months ago by Tade0

Improving the selection check so that it doesn't touch nested editables within non-editable elements solved most of the problems, but there still remain tests that report an Unspecified error, which is troubling.

comment:11 Changed 23 months ago by Tade0

Status: review_failedreview

In hindsight it wasn't a good idea to do manipulate the selection in the selection change handler - not that it didn't work.

Apparently in order not to break e.g. link handling in read-only elements it's better not to do that.

As a last resort measure I'm letting the user place the caret inside the non-editable element, but once a key is pressed the event gets prevented and the whole element is selected.

I put the unit tests in core/selection, because the change applies not only to widgets, but also any other read-only elements. Also, the changes were made in core/selection.js only.

Changes pushed to branch:t/14407.

comment:12 Changed 23 months ago by Tomasz Jakut

Status: reviewreview_failed

Still not working correctly:

  1. Open manual test.
  2. Create widget after the paragraph.
  3. Select some text in the widget (place cursor slightly below the widget and start selecting; ensure that widget is not focused).
  4. Press Backspace.

The selected text in non-editable area is deleted. In the same way it's possible to type a single letter

comment:13 Changed 23 months ago by Tade0

Status: review_failedreview

Verifying also non-collapsed ranges, updated manual test.

Changes pushed to branch:t/14407.

comment:14 Changed 23 months ago by Tomasz Jakut

Status: reviewreview_failed

The issue described in comment is reproducible also in IE8 and IE9 (strangely I didn't manage to reproduce it in IE10). What's more, unit tests for this ticket are failing in all IEs but IE11.

comment:15 Changed 23 months ago by Tade0

Status: review_failedreview

Apparently it's not possible to create such a selection in the unit tests in IE's other than 11 - the selection is changed automatically.

Nevertheless all that was required to make this work on other IE's was to change the browser detection check accordingly.

comment:16 Changed 22 months ago by Tomasz Jakut

Resolution: fixed
Status: reviewclosed

Now LGTM. Merged with git:a3b2b66.

comment:17 Changed 20 months ago by Marek Lewandowski

Summary: Widget can be edited in IE.Non-editable widgets can be edited in IE
Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy