TinyMCE: Assign dummy container during removal #3093
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #3093 +/- ##
==========================================
- Coverage 32.24% 32.23% -0.01%
==========================================
Files 216 216
Lines 6150 6151 +1
Branches 1081 1081
==========================================
Hits 1983 1983
- Misses 3516 3517 +1
Partials 651 651
Continue to review full report at Codecov.
|
|
I will look into fixing this bug on Monday. I think I know exactly why this was introduced. This patch should work fine as a work around until we release 4.7.2 witha fix for the issue. |
youknowriad
approved these changes
Oct 23, 2017
|
@spocke Can you confirm whether this bug has been fixed? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3091
Related: #2787
This pull request seeks to resolve an issue where changing level of a Heading block results in a block error. This began after a TinyMCE upgrade from 4.6.5 to 4.7.1. It is still not clear what specific changes occurred in TinyMCE in this time to introduce the error, since the problematic behavior has existed for some time. Namely, we call to
this.editor.destroywhen the TinyMCE component unmounts for TinyMCE to clean any lingering handlers. However, TinyMCE also tries to remove the container node itself, and when React then proceeds to do the same, an error occurs because the node no longer exists. The changes here assign a dummy container property to the TinyMCE instance while unmounting to trick TinyMCE into removing a different node instead.Ultimately it would be nice if we could perform
destroyclean-up logic without the node being removed, or alternatively not need to explicitly calldestroyand rely on this cleanup to occur as a result of the natural DOM removal.Testing instructions:
Repeat testing instructions from #3091, verifying that no error occurs in changing Heading level.