Opened 6 years ago

Closed 5 years ago

#8345 closed Bug (fixed)

IE : Cursor goes to start of editor body when we bring up link dialog using Link tool bar icon & closing Link dialog using OK Button

Reported by: Satya Minnekanti Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.6.3
Component: Core : Selection Version: 3.5.3
Keywords: IBM IE Cc: Damian, Teresa Monahan

Description

To reproduce the defect:

  1. Open any CK Editor sample and insert a link.
  1. Keep cursor inside the link and click on Link icon in the toolbar.
  1. See that Link dialog comes up
  1. Don't change any values, Close the Link dialog by clicking OK Button.

Expected Result: Link dialog is closed and Link gets selected.

Actual Result: Link dialog is closed but cursor going to start of editor body. This will cause huge inconvenience to users if they want to edit a document with huge content in it.

Attachments (2)

8345.patch (9.6 KB) - added by Garry Yao 6 years ago.
8345_2.patch (3.0 KB) - added by Garry Yao 5 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 6 years ago by Jakub Ś

Keywords: IE added
Status: newconfirmed
Version: 3.5.3

Reproducible in IE browsers from CKEditor 3.5.3 rev [6459]

comment:2 Changed 6 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.2

Changed 6 years ago by Garry Yao

Attachment: 8345.patch added

comment:3 Changed 6 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedreview

I'd like to find a shortcut workaround for this issue, while it doesn't looks simple, so a bit of drastic change is proposed here, to coordinate the locked selection feature and saved range workaround in IE. The following tickets must be considered when reviewing:

comment:4 Changed 6 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.2

Considering that this is not a recent regressions and that the patch proposes a very drastic change, we'll not be able to accommodate this fix into the 3.6.2.

comment:5 Changed 6 years ago by Frederico Caldeira Knabben

A basic test for this issue has been created at t/8345:
http://ckeditor.t/tt/8345/1.html

Further tests are needed though for all related tickets.

comment:6 Changed 6 years ago by Garry Yao

Dt updated to include cursor position check after link creation which should be tackled by this ticket as well.

http://ckeditor.t/dt/plugins/link/link.html

comment:7 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewassigned

I'm not able anymore to properly patch the current trunk. Can you please provide an updated patch for it?

Changed 5 years ago by Garry Yao

Attachment: 8345_2.patch added

comment:8 Changed 5 years ago by Garry Yao

Status: assignedreview

I have simplified the patch, the previous patch is over ambitious for this issue, here the real culprit is that link dialog has somehow touched the dom node so the native range restoring failed to work and since it cancel the locked selection as well, the result cursor is all the way wrong, in addition the patch also addresses the following TC:

  1. Load default replacebyclass sample;
  2. Put cursor in the middle of the link and open Link dialog;
  3. Close the dialog with OK button;
  • Actual Result: Cursor position remains;
  • Expected : Selection expands to have the entire link selected.

Let me claim that this issue doesn't affect v4, where we have a different strategy to have locked selection, so it's enough to have the patch v3 dedicated.

comment:9 Changed 5 years ago by Frederico Caldeira Knabben

Component: GeneralCore : Selection
Status: reviewreview_passed

Please have the original "Update locked selection because of the normalized text nodes" comment back in place when committing, because it makes it easier to understand the hack being done there.

comment:10 Changed 5 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.3

comment:11 Changed 5 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [7384].

comment:12 Changed 5 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: closedreopened

The ticket test is failing on all browsers, except IE.

comment:13 Changed 5 years ago by Garry Yao

Resolution: fixed
Status: reopenedclosed

It's a pity that the ticket tc didn't get reviewed, but it's not been normalized in all browsers, so its not a regression.

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