Conversation
…_clearCacheInsideParent` as the former actually did not correctly clear cache starting from before the specified node. Renamed `MapperCache#_clearCacheFromIndex` to `MapperCache#_clearCacheFromCacheIndex` to underline that the index means cache list index, not view index. Renamed `MapperCache#_clearCacheStartingBefore` to `MapperCache#_clearCacheAfter` as the method was incorrectly named and didn't do what it said. Changes in API docs and in-code comments.
| // Since it is not a tracked element, it has to have a parent. | ||
| this._clearCacheInsideParent( ( viewParent as ViewElement ).parent!, ( viewParent as ViewElement ).index! ); |
There was a problem hiding this comment.
This PR includes some renames and changes in comments/API docs, but this is the place where the actual change/fix happens.
Instead of using this._clearCacheStartingBefore( viewNode ) we use this._clearCacheInsideParent( viewNode.parent, viewNode.index ).
It turned out that _clearCacheStartingBefore() does not really do what it says. Instead, this method guarantees only that all cache after the node is cleared, so it's more like _clearCacheAfter(), and hence it was renamed too.
It actually does something in the middle, because it takes the cached mapping after the node, and clears from there including that mapping. For simple structures this is the same as "clear cache before", however for more complex structures, like from the example:
<p>ab<span>^cd<em><b>e</b>f</em></span>gh</p>
It didn't clear starting from before <span>. Instead it clears starting from "f" --- more or less.
Hence, it was incorrectly used here. Instead of using the method now renamed to clearCacheAfter we invoke _clearCacheInsideParent which correctly clears cache from given view position.
| // | ||
| if ( !this._cachedMapping.has( viewParent ) ) { | ||
| this._clearCacheStartingBefore( viewParent ); | ||
| this._clearCacheInsideParent( viewParent.parent!, viewParent.index! ); |
There was a problem hiding this comment.
This is another place where the same fix was introduced, although given the latest changes in Mapper it seems that this code may not be reachable outside of artificial unit tests crafted for MapperCache. As in "TODO" above, it may be a dead code.
|
This fix leads to another +3-4% of performance overhead for certain scenarios (inline styles and tables). |
2 participants
Suggested merge commit message (convention)
Fix (engine): Fixed editor crash that happened in a specific scenario, when editing heavily formatted text, text with multiple comments, or text with comments and formatting. Closes #18727.
Internal (engine): Renames in
MapperCachemethods so names correctly reflect what the methods do.