Opened 5 years ago

Closed 5 years ago

#11500 closed Bug (fixed)

CKEditor on [Webkit/Blink] drops cursor when changes in one editor update another

Reported by: David Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.4.0
Component: General Version: 4.0 Beta
Keywords: IBM Blink Cc: david_karam@…, fiedler@…

Description (last modified by Jakub Ś)

  1. Go to http://ckeditor.com/demo#inline on a Webkit-based browser
  2. Bind contents of editor1 and editor2 on a key listener
    CKEDITOR.instances.editor1.on("key", function() { CKEDITOR.instances.editor2.setData(CKEDITOR.instances.editor1.getData()) });
    
  3. Update editor1 text.

Expected: editor2 updates. editor1 cursor remains at original place. Observed: editor2 updates. editor1 cursor goes back to start of editor.

Screencast attached. Firefox works fine.

Attachments (3)

CK Editor - Webkit issue.mp4 (1.2 MB) - added by David 5 years ago.
Screencast of issue
11500.html (2.1 KB) - added by Piotrek Koszuliński 5 years ago.
Bug still exists.mp4 (328.4 KB) - added by David 5 years ago.
Bug reproducible after incorporating fix

Download all attachments as: .zip

Change History (17)

Changed 5 years ago by David

Screencast of issue

comment:1 Changed 5 years ago by David

Cc: david_karam@… added

comment:2 Changed 5 years ago by David

Further debugging found the issue in CKEditor in selection.js - line 452.

Webkit's selection will mess up after the data loading.

CKEDITOR.env.webkit && editor.on( 'setData', clearSelection );

Would it be enough to only clear the selection if the selection is in the current element?

Is there any possible workaround that can be provided as this issue is blocking us.

comment:3 Changed 5 years ago by mffiedler

Cc: fiedler@… added

Add CC

comment:4 Changed 5 years ago by Jakub Ś

Description: modified (diff)
Keywords: IBM Blink added; ibm removed
Status: newconfirmed
Summary: CKEditor on Webkit drops cursor when changes in one editor update anotherCKEditor on [Webkit/Blink] drops cursor when changes in one editor update another
Version: 4.3.24.0 Beta

Problem can be reproduced from CKEditor 4.0 beta in Webkit and Blink in inline editors only.

comment:5 Changed 5 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.3.3
Owner: set to Piotrek Koszuliński
Status: confirmedassigned

I pushed t/11500 with a proposal of patch that solves the problem. I'm not sure if check shouldn't be moved to removeAllRanges() though.

comment:6 in reply to:  5 Changed 5 years ago by Olek Nowodziński

Replying to Reinmar:

I pushed t/11500 with a proposal of patch that solves the problem. I'm not sure if check shouldn't be moved to removeAllRanges() though.

I tested your idea for a while

removeAllRanges: function() {
	if ( this.getType() == CKEDITOR.SELECTION_NONE )
		return;

	...
}

...and it looks fine. IIRC, in real-world usage CKEDITOR.SELECTION_NONE is only possible when editor never got focus or setData is being executed. If that's true, I'm OK with the solution because I see no pitfalls.

Changed 5 years ago by Piotrek Koszuliński

Attachment: 11500.html added

comment:7 Changed 5 years ago by Piotrek Koszuliński

Status: assignedreview

Pushed branch:t/11500b on dev and tests.

Last edited 5 years ago by Piotrek Koszuliński (previous) (diff)

comment:8 Changed 5 years ago by Olek Nowodziński

Status: reviewreview_passed

comment:9 Changed 5 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on master with git:9d666dc on dev and 23400e2 on tests.

comment:10 Changed 5 years ago by David

This still does not work properly.

Add a listener from editor1 to editor2 and from editor2 to editor1 (this is our use case).

  1. Type "a" in editor1. Both editors have "a".
  2. Type "b" in editor2. Both editors have "ab".
  3. Type "c" in editor 1. Both editors have "abc".

Further typing in editor1 will show the same cursor issue. Attached a screencast.

Changed 5 years ago by David

Attachment: Bug still exists.mp4 added

Bug reproducible after incorporating fix

comment:11 Changed 5 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.3.3CKEditor 4.3.4
Resolution: fixed
Status: closedreopened

Unfortunately I can confirm this. The TC is simple:

  1. Focus one of inline editors.
  2. Start typing in the second one.

The reason why this happens is that the first editor, when blurred locks selection and this code:

function clearSelection() {
	var sel = editor.getSelection();
	sel && sel.removeAllRanges();
}

Gets that locked selection, which is not empty. This code should get real selection (getSelection( 1 )) and that fixes the issue.

However, we should ask why the selection stays locked if there's a setData() call which we know will invalidate the locked selection. Surprise, surprise, this thread moves us straight to http://dev.ckeditor.com/ticket/5217#comment:32 which we found during 4.3.3's testing phase. So both tickets will have the same fix apparently.

comment:12 Changed 5 years ago by Piotrek Koszuliński

Status: reopenedconfirmed

comment:13 Changed 5 years ago by Piotrek Koszuliński

Status: confirmedassigned

Patch for #5217 fixes this issue.

comment:14 Changed 5 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.3.4CKEditor 4.4
Resolution: fixed
Status: assignedclosed

Fixed by #5217 on major.

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