Cannot read property 'blur' of null in focusManager

mlocati commented 4 years ago

Type of report

Bug

Provide detailed reproduction steps (if any)

Reported in https://github.com/concrete5/concrete5/issues/8271 but it's very hard to be replicated. Basically, I think that this can be replicated with these steps:

open a CKEditor instance
hit the Link toolbar button
click on an input field (for example Display Text) when there's a hook in the page that destroys CKEditor when an input field is focuses.

Expected result

No errors occur.

Actual result

Error logged in the developer console:

ckeditor.js:258 Uncaught TypeError: Cannot read property 'blur' of null
    at CKEDITOR.focusManager.remove (ckeditor.js:258)
    at N (ckeditor.js:595)
    at CKEDITOR.dialog.hide (ckeditor.js:616)
    at Object.<anonymous> (ckeditor.js:648)
    at Object.l (ckeditor.js:10)
    at Object.fire (ckeditor.js:12)
    at a.destroy (ckeditor.js:289)

Other details

Browser: any
OS: any
CKEditor version: 4.12
Installed CKEditor plugins: a11yhelp, about, ajax, autocomplete, autogrow, autolink, basicstyles, bidi, blockquote, button, clipboard, colorbutton, colordialog, concrete5filemanager, concrete5inline, concrete5link, concrete5styles, concrete5uploadimage, contextmenu, dialogadvtab, dialogui, dialog, divarea, div, elementspath, emoji, enterkey, entities, fakeobjects, filebrowser, filetools, find, flash, floatingspace, floatpanel, font, format, horizontalrule, htmlwriter, iframe, image2, image, indentblock, indentlist, indent, justify, language, lineutils, link, listblock, liststyle, list, magicline, maximize, menubutton, menu, newpage, normalizeonchange, notificationaggregator, notification, pagebreak, panelbutton, panel, pastefromword, pastetext, placeholder, popup, preview, print, removeformat, resize, richcombo, save, scayt, selectall, showblocks, showborders, smiley, sourcearea, sourcedialog, specialchar, stylescombo, tableresize, tableselection, tabletools, table, tab, textmatch, textwatcher, toolbar, undo, uploadimage, uploadwidget, widgetselection, widget, wsc, wysiwygarea, xml

f1ames commented 4 years ago

Hello @mlocati, thanks for reporting. I wonder what is your use case (seen .gif from https://github.com/concrete5/concrete5/issues/8271 but I'm not familiar with the system/integration shown there).

click on an input field (for example Display Text) when there's a hook in the page that destroys CKEditor when an input field is focuses.

Can you elaborate more on this, I can imagine this causes an issue, but destroying editor while its dialog input gets focused doesn't seem to make sense 🤔 Is there any particular result which you are trying to achieve with such behaviour?

mlocati commented 4 years ago

Can you elaborate more on this, I can imagine this causes an issue, but destroying editor while its dialog input gets focused doesn't seem to make sense

Yep, that's sure! Indeed that shouldn't occur (see https://github.com/concrete5/concrete5/issues/8271#issuecomment-558999708), but also the TypeError fixed by #3687 shouldn't occur :wink:

mlocati commented 4 years ago

The problem is that

the CKEditor instance is created inside an X-editable popup
the link dialog is appended to the document body element
when the user clicks the dialog, X-editable closes the CKEditor instance because the popup loses the focus

So, we'd need to:

add the CKEditor dialogs to the popup or
disable that feature of X-editable

I think that it's not possible to do 1, right? See https://github.com/ckeditor/ckeditor4/blob/major/plugins/dialog/plugin.js#L871

f1ames commented 4 years ago

Hi @mlocati, thanks for your insights.

add the CKEditor dialogs to the popup

Indeed it might be cumbersome to try to hack it somehow, it's doable but you will have to play with dialog internals which could be quite complex.

disable that feature of X-editable

Not sure what is X-editable (I see some Bootstrap stuff - https://vitalets.github.io/x-editable/). Looking into it, probably that would by the easier way to workaround this issue. And I think you would get rid of the UI/UX issue which is related to that:

destroying editor while its dialog input gets focused doesn't seem to make sense

mlocati commented 4 years ago

Yep, https://github.com/concrete5/concrete5/pull/8272 fixes the issue, but I still think it may be somehow useful to fix this #3686 (given also that the fix if very light - see #3687)

Dumluregn commented 4 years ago

Hello, thank you for your contribution to our repo! We will take a look at it. Still, as the issue is not reproducible in the editor itself, it seems that fixing concrete5/concrete5#8272 is a better way to deal with this issue (especially that #3687 doesn't fix the issue but just doesn't throw error).

douglyuckling commented 3 years ago

Also running into this today.

Here's an easy way to replicate it, by the way:

Go to the demo site for CKEditor4: https://ckeditor.com/ckeditor-4/demo/#article
Click the link button to open the Link dialog (or any other dialog, I assume)
Open your browser's JavaScript console and invoke destroy() on the editor instance. (Optionally, save a reference to the instance first for replicating a further issue later.)
```
theEditor = CKEDITOR.instances.ckdemo
theEditor.destroy()
```

You'll get a stack trace in the console, and although the editor itself is gone the dialog's backdrop stays around.

Also, if you invoke theEditor.destroy() a second time, the JS seems to get stuck in an infinite loop—the page becomes unresponsive with high CPU usage and stops responding to events.

ckeditor / ckeditor4