Testcase takes 22s to copy-paste from browser to a text editor. With a tiny modification, it takes 200ms.
Categories
(Core :: Web Painting, enhancement)
Tracking
()
People
(Reporter: mayankleoboy1, Unassigned)
References
Details
Attachments
(3 files)
- Type the following in a new tab : " data:text/html, <html style contenteditable spellcheck="false"> " . This will create a text editor type thingy.
- Open the slow testcase in the browser
- Select all the text and copy it (Ctrl+A, Ctrl + C)
- Paste the text into the text-editor from #1
- Now repeat the steps with the fast testcase
Fast: https://share.firefox.dev/4fDxgIg (174ms)
Slow: https://share.firefox.dev/4daAYaT (22s)
The fast testcase is the original testcase. To make it slow, I deleted all the <div> and </div> tags.
Maybe something to improve for the slow testcase?
|
Reporter | |
Comment 1•3 months ago
|
||
|
|
Reporter | |
Comment 2•3 months ago
|
||
|
|
Reporter | |
Comment 3•3 months ago
|
||
:jjaschke, I am ni? you as you may have some opinion on this.
|
||
Comment 4•3 months ago
|
||
Interesting case, thank you for posting. The 'slow' case is a node with a lot of child nodes, hence ComputeIndexOf() becomes slow and shows up in the profile. I wrote some technical details in Bug 1503581 comment 14.
Since I have looking into bug 1503581, I tried this test case with that WIP patch applied. It turns out that said patch adds roughly one order of magnitude to the slow case (the fast testcase stays the same. It doesn't use the cache because of the 1k child node threshold). The reason is, that while pasting, the document's generation changes permanently, forcing the cache to be rebuilt constantly.
I think it may be possible to add some batching there:
While inserting, we're only adding nodes into a single parent node. So, theoretically, we don't have to clear the cache (this is more a question than a statement!). Note that I have limited knowledge of the Editor internals.
If we would add batching to the AutoPlaceholderBatch which would clear the cache once and then prohibit clearing the cache, we would keep the cache alive during the paste operation.
I gave that a try, and the slow test case was about as fast as the fast one (<1s).
However, IIUC inserting nodes during paste could trigger DOM mutations, which would currently be handled during paste. These would have to be batched as well.
Masayuki, what do you think about this (also bug 1503581)? I'm sure I've missed many edge cases and I don't know if AutoPlaceholderBatch is the right place to add the batching. Do you think it makes sense to look deeper into this?
|
||
Comment 5•3 months ago
|
||
Well, I think it's fine to put it into HTMLEditor::HTMLWithContextInserter::InsertContents because the insertion occur sequentially. It might break some cases if legacy mutation event listeners intercepts something. However, in such tricky case, web apps should charge with the result by themselves. (FYI: Chrome/Safari does not dispatch events while their builtin editor touches the DOM.)
Description
•