Opened 4 years ago

Closed 2 years ago

#13362 closed Bug (fixed)

Wrong focus after creating link in caption using enhanced image widget

Reported by: tomm Owned by: kkrzton
Priority: Normal Milestone: CKEditor 4.5.11
Component: General Version:
Keywords: Blink Webkit Cc:

Description

When you create a link in the caption text the text cursor in the editor instance jumps to the start of the content.

Steps to reproduce:

  1. Go to http://ckeditor.com/demo#widgets and find the enhanced image demo
  1. Create a link in the caption of the inserted image in the demo
  1. The text cursor jumps to the first line in the editor instance

Browser/OS:

Safari 8.0.6 on OSX 10.10.3.

Attachments (1)

Screen Shot 2015-06-04 at 11.25.24.png (229.7 KB) - added by tomm 4 years ago.
Screenshot of the problem

Download all attachments as: .zip

Change History (12)

Changed 4 years ago by tomm

Screenshot of the problem

comment:1 Changed 4 years ago by Jakub Ś

Keywords: Blink Webkit added
Status: newconfirmed
Summary: Wrong focus after creating link in caption using enhanced image widget in SafariWrong focus after creating link in caption using enhanced image widget

Problem can be reproduced in Blink and Webkit browsers from CKEditor 4.3 beta.

comment:2 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.7

comment:3 Changed 3 years ago by kkrzton

Owner: set to kkrzton
Status: confirmedassigned

comment:4 Changed 3 years ago by kkrzton

When any dialog is closed, the editor is refocused. The native focus() method is called on the root element (body) of the editor. Normally when element is already focused and focus() is called, nothing happens (and focus/selection inside this element is not affected).

The widgets are contenteditable=false which makes them non-focusable (I suppose). But inside them, there are other elements that are contenteditable=true (and so can be focused).

The problem with Blink/Webkit is that nested contenteditable=false somehow breaks "focus scope" and parent element is not consider focused when nested element with contenteditable=false has focus inside. So it causes focus (and caret) to move to the root container.

jsfiddle with nested contenteditables

Last edited 3 years ago by kkrzton (previous) (diff)

comment:5 Changed 3 years ago by kkrzton

There is a fix to handle proper widgets focusing on Webkit/Blink 16548c8 (to solve the problem with nested contenteditables - see previous comment).

Note that while dialog is open focus always points to editor root so widget focus gets lost and needs to be stored - that's the main idea of this fix.

So the above fix saves focus context when selection changes (selection.checkSelectionChange). When dialog is closed, the focus is restored in editor - based on previously saved or currently active value. The focus() method is called after closing/hiding dialog (dialog.hide method).

The link dialog uses onOk method (which is executed before hiding) to process dialog input and insert link. When inserting link the range.select() method is used to add selection to the inserted link. Adding selection means that selection has changed (selection.checkSelectionChange) so new focus context is saved. Since dialog is not yet hidden, focus points to editor root which means it overrides previously stored value (pointing to widget). After that, focus() is executed (hiding dialog) and stored value now points to editor root. The root gets focused instead of previously selected widget.

comment:6 Changed 3 years ago by kkrzton

Status: assignedreview

Changes in t/13362

comment:7 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.7CKEditor 4.5.8

comment:8 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.8CKEditor 4.5.9

comment:9 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.9CKEditor 4.5.10

comment:10 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.10CKEditor 4.5.11

Moving tickets to the next milestone.

comment:11 Changed 2 years ago by Tade0

Resolution: fixed
Status: reviewclosed

I think this should suffice for a fix.

Fixed with git:3933d6c1cc861325f4a030e959526a6fafbbb3f1

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