Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heading Block: Unhandled DOMException when changing heading level #3091

Closed
mcsf opened this issue Oct 20, 2017 · 4 comments · Fixed by #3093
Closed

Heading Block: Unhandled DOMException when changing heading level #3091

mcsf opened this issue Oct 20, 2017 · 4 comments · Fixed by #3093
Labels
[Feature] Blocks Overall functionality of blocks [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended

Comments

@mcsf
Copy link
Contributor

mcsf commented Oct 20, 2017

Issue Overview

Can't change the heading level (nodeName) of a heading block; see screencast below.

screen shot 2017-10-20 at 17 04 14

Console trace:

react-dom.min.cc6ec027.js:23 DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.
    at removeChild (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:201:465)
    at f (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:33:1)
    at commitDeletion (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:36:302)
    at c (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:15:422)
    at k (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:19:411)
    at Q (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:20:355)
    at batchedUpdates (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:26:228)
    at Ed (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:73:389)
    at kc (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:66:88)
    at Object.batchedUpdates (http://127.0.0.1:8000/wp-content/plugins/gutenberg/vendor/react-dom.min.cc6ec027.js:121:469)

Steps to Reproduce

heading-block-fail

@mcsf mcsf added [Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended labels Oct 20, 2017
@mcsf mcsf changed the title Heading Block: Unhandled error when changing heading level Heading Block: Unhandled DOMException when changing heading level Oct 20, 2017
@youknowriad
Copy link
Contributor

We're forcing the TinyMCE component to remount (a new key prop) when we change the nodeName which seems logical. But for a reason I didn't find yet, this stopped working properly and is throwing the error above

@mcsf mcsf added the [Priority] High Used to indicate top priority items that need quick attention label Oct 20, 2017
@aduth
Copy link
Member

aduth commented Oct 20, 2017

Started as of TinyMCE upgrade in #2787 38bcd84 cc @iseulde

@aduth
Copy link
Member

aduth commented Oct 20, 2017

My guess is that editor.destroy is now removing the node itself:

this.editor.destroy();

Commenting this line resolves the issues; it might be enough to just remove this, though we'd want to check if there are any listeners we'd need to be concerned about that aren't being cleaned up through the DOM removal alone.

@spocke
Copy link

spocke commented Oct 22, 2017

#3093 would work fine until we release a fix in 4.7.2 we have still a few things we need to fix before we can release that one. The editor container is to be removed in iframe mode only the headless mode by setting theme: false is now supported by both inline/iframe and that was a recent change. That caused code to be shared between the modes that accidently assigned the editorContainer to the target for inline mode and that causes it to be removed when ”destory” is called. Destory or rather ”remove” should be called when you want to detach a editor from a div since it will otherwise have lingering event handles and also the editor instance still in the EditorManager collection and that would mean that the GC can’t remove anything that editor is referencing and that could be a lot of things and produce a memory leak. We will do a fix for this bug on Monday and push to public master but a proper release is likely to be done later not currently sure when need to look into that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants