Opened 9 years ago
Closed 9 years 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
- Focus is on the editor, not blur
- Click jQuery UI Dialog cancel button ,then fire close event:
close: function () { editor.destroy(); editor = null; }
- 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)
Change History (13)
Changed 9 years ago by
| Attachment: | jquerydialog.html added |
|---|
comment:1 Changed 9 years ago by
| Status: | new → confirmed |
|---|
comment:2 Changed 9 years ago by
| Keywords: | Support added |
|---|
Changed 9 years ago by
| Attachment: | inlinebycode2.html added |
|---|
comment:3 Changed 9 years ago by
| Milestone: | → CKEditor 4.7.0 |
|---|
Changed 9 years ago by
| Attachment: | bug_focusManager.html added |
|---|
Illustrate focusManager trying to blur a destroyed editor
comment:4 Changed 9 years ago by
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
- Use a blink browser (Chrome or Opera).
- See this demo: https://jsfiddle.net/rbo0o8ze/, or download attachment (bug_focusManager.html).
- Once editor loaded and focused click "delete".
- 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.
comment:5 Changed 9 years ago by
| Owner: | set to Tade0 |
|---|---|
| Status: | confirmed → assigned |
comment:6 Changed 9 years ago by
| Status: | assigned → review |
|---|
Indeed this check was missing. Thank you, denisname.
Added a unit test for this case. Changes pushed to branch:t/16825.
comment:7 Changed 9 years ago by
| Status: | review → review_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.
comment:8 Changed 9 years ago by
| Status: | review_failed → review |
|---|
Updated unit test, added manual test, rebased with major.
Changes pushed to branch:t/16825.
comment:9 Changed 9 years ago by
For reference, first bad commit was git:2c0d9802bd809bb3c28c86ea8ff911f351d6ae18.
comment:10 Changed 9 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review → closed |
| 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.

Problem can be reproduced from CKEditor 4.6.2 in Chrome only.