Opened 8 years ago
Closed 8 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 8 years ago by
Attachment: | jquerydialog.html added |
---|
comment:1 Changed 8 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 8 years ago by
Keywords: | Support added |
---|
Changed 8 years ago by
Attachment: | inlinebycode2.html added |
---|
comment:3 Changed 8 years ago by
Milestone: | → CKEditor 4.7.0 |
---|
Changed 8 years ago by
Attachment: | bug_focusManager.html added |
---|
Illustrate focusManager trying to blur a destroyed editor
comment:4 Changed 8 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 8 years ago by
Owner: | set to Tade0 |
---|---|
Status: | confirmed → assigned |
comment:6 Changed 8 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 8 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 8 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 8 years ago by
For reference, first bad commit was git:2c0d9802bd809bb3c28c86ea8ff911f351d6ae18.
comment:10 Changed 8 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.