- Open a large doc (>100k)
- Hold down a key to type lots of chars quickly (~100).
- Press undo
We appear to be trying to render the cursor after each transaction in the undo stack is committed. This should be debounced.
Description
Description
Details
Details
Related Changes in Gerrit:
Customize query in gerrit
Related Objects
Related Objects
Event Timeline
Comment Actions
Change 623411 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] ve.ce.Surface: Debounce onModelDocumentUpdate
Comment Actions
Change 623411 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] ve.ce.Surface: Debounce onModelDocumentUpdate
Comment Actions
Change 623875 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (b49db9a5b)
Comment Actions
Change 623875 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (9534694b4)
Comment Actions
Change 629471 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] Follow-up I78041b9b4: Debounce just showModelSelection
Comment Actions
All the extra calls appear to come from:
/** * Handle documentUpdate events on the surface model. */ ve.ce.Surface.prototype.onModelDocumentUpdate = function () { var surface = this; if ( this.contentBranchNodeChanged ) { // Update the selection state from model this.onModelSelect(); }
At the very least this could probably be debounced, although I'm struggling to see when it is ever necessary. cc @dchan
Comment Actions
It's done because a model change within a ContentBranchNode gets propagated to the view by replacing the entire contents of the CBN, which resets the native selection to the start of the CBN.
Comment Actions
Change 809052 had a related patch set uploaded (by Divec; author: Divec):
[VisualEditor/VisualEditor@master] Only update cursor holders if they're not already correct.
Comment Actions
Change 809093 had a related patch set uploaded (by Divec; author: Divec):
[VisualEditor/VisualEditor@master] Use TransactionSquasher to speed up undo/redo
Comment Actions
On a quick test I see a ~5x speedup, testing on 809052 and 909093 above.
Test page (158k): https://en.wikipedia.org/wiki/Government_policies_and_the_subprime_mortgage_crisis
Test: Quickly type a few lines of English, then repeatedly press CTRL+Z to undo.
Profile results (running on live): average 198ms spent in keydown handler
Profile results (running on new patches): average 38ms spent in keydown handler
Comment Actions
Change 629471 abandoned by Esanders:
[VisualEditor/VisualEditor@master] Follow-up I78041b9b4: Debounce just showModelSelection
Reason:
Comment Actions
Change 809052 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Only update cursor holders if they're not already correct.
Comment Actions
Change 981374 had a related patch set uploaded (by Esanders; author: Esanders):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (402570007)
Comment Actions
Change 981374 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (402570007)
Comment Actions
@dchan: Removing task assignee as this open task has been assigned for more than two years - See the email sent to task assignee on October 11th.
Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome! :)
If this task has been resolved in the meantime, or should not be worked on by anybody ("declined"), please update its task status via "Add Action… 🡒 Change Status".
Also see https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator. Thanks!