Opened 6 months ago

Closed 4 months ago

#16825 closed Bug (fixed)

[Chrome] Error thrown when destroying focused inline editor

Reported by: lb009 Owned by: Tade0
Priority: Normal Milestone: CKEditor 4.7.0
Component: Core : Focus Version: 4.6.2
Keywords: Support Cc:

Description

Steps to reproduce

  1. Focus is on the editor, not blur
  2. Click jQuery UI Dialog cancel button ,then fire close event:
       close: function () 
       {
           editor.destroy();
           editor = null;
       }
    
  1. Uncaught TypeError: Cannot read property 'isInline' of null

at CKEDITOR.focusManager.d (ckeditor.js:238)

at CKEDITOR.focusManager.<anonymous> (ckeditor.js:238)

at ckeditor.js:28

Actual result

After call editor.destroy(),editor.editable() return null

// Blink browsers leave selection in `[contenteditable=true]`
// when it's blurred and it's neccessary to remove it manually for inline editor. (#13446)

if ( CKEDITOR.env.chrome && editor.editable().isInline() ) {
editor.window.$.getSelection().removeAllRanges();
}

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

Chrome 55.0.2883.87 m

CKEditor 4.6.2

Attachments (3)

jquerydialog.html (2.2 KB) - added by Jakub Ś 6 months ago.
inlinebycode2.html (2.6 KB) - added by Jakub Ś 6 months ago.
bug_focusManager.html (555 bytes) - added by Denis 6 months ago.
Illustrate focusManager trying to blur a destroyed editor

Download all attachments as: .zip

Change History (13)

Changed 6 months ago by Jakub Ś

Attachment: jquerydialog.html added

comment:1 Changed 6 months ago by Jakub Ś

Status: newconfirmed

Problem can be reproduced from CKEditor 4.6.2 in Chrome only.

Another test case for inline sample and shared spaces plugin:

  1. Load attached sample file inlinebycode2.html.
  2. Once the page is loaded, click into right editor and wait until it is destroyed.
  3. Click into left editor

Result: Error is thrown

Last edited 6 months ago by Jakub Ś (previous) (diff)

comment:2 Changed 6 months ago by Jakub Ś

Keywords: Support added

Changed 6 months ago by Jakub Ś

Attachment: inlinebycode2.html added

comment:3 Changed 6 months ago by Jakub Ś

Milestone: CKEditor 4.7.0

Changed 6 months ago by Denis

Attachment: bug_focusManager.html added

Illustrate focusManager trying to blur a destroyed editor

comment:4 Changed 6 months ago by Denis

Got the same bug. Here are possible reason and solution, and also a workaround.

The Focus Manager focus and blur editors "asynchronously" with some delay. When destroying a CKEditor with focus, this result in an attempt to blur a released editor and produce an exception.

Steps to Reproduce

  1. Use a blink browser (Chrome or Opera).
  2. See this demo: https://jsfiddle.net/rbo0o8ze/, or download attachment (bug_focusManager.html).
  3. Once editor loaded and focused click "delete".
  4. See error in console.

Actual Result

Exception is thrown:

Uncaught TypeError: Cannot read property 'isInline' of null
    at CKEDITOR.focusManager.doBlur (focusmanager.js:161)
    at CKEDITOR.focusManager.<anonymous> (focusmanager.js:180)
    at tools.js:578

Expected Result

Don't throw...

Workaround

Problem can be avoid by forcing the editor to blur without delay before destroy: editor.focusManager.blur(true). See "Steps to Reproduce" demo.

Bug Reason

Error is caused in doBlur function contained in blur method of focusManager. See: https://github.com/ckeditor/ckeditor-dev/blob/2db0ce3e25f9c97404e2be107e1250a78254ddd7/core/focusmanager.js#L161-L163.

When the editor is destroyed, editor.editable() is set to null. See https://github.com/ckeditor/ckeditor-dev/blob/2db0ce3e25f9c97404e2be107e1250a78254ddd7/core/editor.js#L807.

Hence, editor.editable().isInline() can only fail.

The CKEDITOR.env.chrome test explains why this only occurs on blink browsers.

Proposed Solution

I propose to make a pull request changing the line:

if ( CKEDITOR.env.chrome && editor.editable().isInline() ) {

by

if ( CKEDITOR.env.chrome && editor.editable() && editor.editable().isInline() ) {

This make the code work as expected.

Last edited 6 months ago by Denis (previous) (diff)

comment:5 Changed 5 months ago by Tade0

Owner: set to Tade0
Status: confirmedassigned

comment:6 Changed 5 months ago by Tade0

Status: assignedreview

Indeed this check was missing. Thank you, denisname.

Added a unit test for this case. Changes pushed to branch:t/16825.

comment:7 Changed 5 months ago by Marek Lewandowski

Status: reviewreview_failed
  • Given unit test doesn't test this bug. When checked out major version of core\focusmanager.js, test still passes.
  • I'm also missing manual test.

I have rebased the branch to the latest major.

Last edited 5 months ago by Marek Lewandowski (previous) (diff)

comment:8 Changed 5 months ago by Tade0

Status: review_failedreview

Updated unit test, added manual test, rebased with major.

Changes pushed to branch:t/16825.

comment:9 Changed 4 months ago by Marek Lewandowski

For reference, first bad commit was git:2c0d9802bd809bb3c28c86ea8ff911f351d6ae18.

comment:10 Changed 4 months ago by Marek Lewandowski

Resolution: fixed
Status: reviewclosed
Summary: Uncaught TypeError: Cannot read property 'isInline' of null[Chrome] Error thrown when destroying focused inline editor

Added some changes, mainly to tests.

Fixed with git:33bd6288af, merged to major.

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