Opened 5 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 Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.6.3
Component: Core : Selection Version: 3.5.3
Keywords: IBM IE Cc: damo, tmonahan

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 5 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 5 years ago by j.swiderski

  • Keywords IE added
  • Status changed from new to confirmed
  • Version set to 3.5.3

Reproducible in IE browsers from CKEditor 3.5.3 rev [6459]

comment:2 Changed 5 years ago by fredck

  • Milestone set to CKEditor 3.6.2

Changed 5 years ago by garry.yao

comment:3 Changed 5 years ago by garry.yao

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

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 5 years ago by fredck

  • Milestone CKEditor 3.6.2 deleted

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 5 years ago by fredck

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 5 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 fredck

  • Status changed from review to assigned

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

comment:8 Changed 5 years ago by garry.yao

  • Status changed from assigned to review

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 fredck

  • Component changed from General to Core : Selection
  • Status changed from review to review_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 fredck

  • Milestone set to CKEditor 3.6.3

comment:11 Changed 5 years ago by garry.yao

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

Fixed with [7384].

comment:12 Changed 5 years ago by fredck

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:13 Changed 5 years ago by garry.yao

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

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 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy