[CKEditor Dev] #16825: Uncaught TypeError: Cannot read property 'isInline' of null
CKEditor
-------------------+--------------------------
Reporter: lb009 | Owner:
Type: Bug | Status: new
Priority: Normal | Component: Core : Focus
Version: 4.6.2 | Keywords:
-------------------+--------------------------
== 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;
}
}}}
3.
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
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825>
CKEditor <http://ckeditor.com/>
The text editor for the Internet
CKEditor
Reporter: lb009 | Owner:
Type: Bug | Status: new
Component: Core : Focus | Version: 4.6.2
---------------------------+-------------------
Changes (by j.swiderski):
* Attachment "jquerydialog.html" added.
CKEditor
Reporter: lb009 | Owner:
Type: Bug | Status: confirmed
Priority: Normal | Milestone:
Component: Core : Focus | Version: 4.6.2
---------------------------+-----------------------
Changes (by j.swiderski):
* status: new => confirmed
Comment:
Problem can be reproduced from CKEditor 4.6.2 in Chrome only.
On our support channel we have a similar request concerning ShredSpace
plugin, inline editor and Firefox so this issue might be affecting more
than one browser. Ticket number is 21898
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:1>
CKEditor
Reporter: lb009 | Owner:
Type: Bug | Status: confirmed
Priority: Normal | Milestone:
Component: Core : Focus | Version: 4.6.2
---------------------------+-----------------------
Changes (by j.swiderski):
* keywords: => Support
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:2>
CKEditor
Reporter: lb009 | Owner:
Type: Bug | Status: confirmed
Priority: Normal | Milestone:
Component: Core : Focus | Version: 4.6.2
---------------------------+-----------------------
Changes (by j.swiderski):
* Attachment "inlinebycode2.html" added.
CKEditor
Reporter: lb009 | Owner:
Type: Bug | Status: confirmed
Priority: Normal | Milestone: CKEditor 4.7.0
Component: Core : Focus | Version: 4.6.2
---------------------------+----------------------------
Changes (by j.swiderski):
* milestone: => CKEditor 4.7.0
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:3>
CKEditor
Reporter: lb009 | Owner:
Type: Bug | Status: confirmed
Priority: Normal | Milestone: CKEditor 4.7.0
---------------------------+----------------------------
Changes (by denisname):
* Attachment "bug_focusManager.html" added.
Illustrate focusManager trying to blur a destroyed editor
CKEditor
Reporter: lb009 | Owner:
Type: Bug | Status: confirmed
Priority: Normal | Milestone: CKEditor 4.7.0
---------------------------+----------------------------
Comment (by denisname):
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 ==
0. Use a blink browser (Chrome or Opera).
1. See this demo: https://jsfiddle.net/rbo0o8ze/, or download attachment
(bug_focusManager.html).
2. Once editor loaded and ''focused'' click "delete".
3. 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.
Once the editor destroyed, editor.editable() is null. 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.
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:4>
CKEditor
Reporter: lb009 | Owner: Tade0
Type: Bug | Status: assigned
Priority: Normal | Milestone: CKEditor 4.7.0
---------------------------+----------------------------
Changes (by Tade0):
* owner: => Tade0
* status: confirmed => assigned
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:5>
CKEditor
Reporter: lb009 | Owner: Tade0
Priority: Normal | Milestone: CKEditor 4.7.0
---------------------------+----------------------------
Changes (by Tade0):
* status: assigned => review
Comment:
Indeed this check was missing. Thank you, `denisname`.
Added a unit test for this case. Changes pushed to branch:t/16825.
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:6>
CKEditor
Reporter: lb009 | Owner: Tade0
Priority: Normal | Milestone: CKEditor 4.7.0
---------------------------+----------------------------
Changes (by m.lewandowski):
* status: review => review_failed
Comment:
* 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.
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:7>
CKEditor
Reporter: lb009 | Owner: Tade0
Type: Bug | Status: review
---------------------------+----------------------------
Changes (by Tade0):
* status: review_failed => review
Comment:
Updated unit test, added manual test, rebased with `major`.
Changes pushed to branch:t/16825.
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:8>
CKEditor
Reporter: lb009 | Owner: Tade0
Type: Bug | Status: review
Priority: Normal | Milestone: CKEditor 4.7.0
---------------------------+----------------------------
Comment (by m.lewandowski):
For reference, first bad commit was
git:2c0d9802bd809bb3c28c86ea8ff911f351d6ae18.
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:9>
CKEditor
---------------------------+----------------------------
Reporter: lb009 | Owner: Tade0
Priority: Normal | Milestone: CKEditor 4.7.0
---------------------------+----------------------------
Changes (by m.lewandowski):
* status: review => closed
* resolution: => fixed
Comment:
Added some changes, mainly to tests.
Fixed with git:33bd6288af, merged to major.
--
Ticket URL: <http://dev.ckeditor.com/ticket/16825#comment:10>