Rich text: resolve owned event listeners once instead of per subscriber - #80605
Open
ellatrix wants to merge 4 commits into
Open
Rich text: resolve owned event listeners once instead of per subscriber#80605ellatrix wants to merge 4 commits into
ellatrix wants to merge 4 commits into
Conversation
Rewrite subscribeOwnedListener to share one delegated listener per document, event type and phase, with a dispatch that finds the owning elements by walking up from the event target, the selection anchor and the focused element, and firing their callbacks. It applies the same contains-or-ownsSelection check as before, only on the nodes that could pass it, instead of every subscriber running it on every event (O(blocks) per keystroke). Move selectionchange and ensureSelectionSync onto subscribeOwnedListener too, so they ride the same per-element dispatch and stay ordered ahead of the other owned listeners for the element, keeping the record synced before they run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrG35JQKJBX1iNof5wtfjN
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +68 B (0%) Total Size: 7.75 MB 📦 View Changed
|
Every subscribeOwnedListener consumer is an editing or selection event, so the owner is the element that owns the selection, and the event target is always the caret. That makes the target chain and the contains-target check redundant: walk up only from the selection anchor (the element that contains the selection) and the focused element (the case where an editable is focused but the selection is elsewhere), testing ownsSelection alone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrG35JQKJBX1iNof5wtfjN
The owner always contains the selection anchor, so one walk up from the anchor finds it; fall back to the focused element only when there is no selection. This drops the second chain and the dedup set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrG35JQKJBX1iNof5wtfjN
It had a single caller and closed over elements and ownerDocument already, so folding it in drops the indirection and the parameters. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrG35JQKJBX1iNof5wtfjN
This file contains hidden or 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
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.
What?
Rewrites
subscribeOwnedListenerso it resolves which element owns an event once per event, instead of every subscriber checking whether it owns the event on every event.Alternative to #80517 (same fix, simpler dispatch).
Why?
Typing performance regression from
editableRoot(#79105). Each editable subscribes owned listeners bound to the document, so all of them run on every event and each callsownsSelection. With ~8 keydown subscriptions per editable and an editable per block, a keystroke in a 1250-block post is ~10,000ownsSelectioncalls.Note: this is what actually moves the metric. #80549 (avoid the forced layout) and #80548 (reorder the checks) both flatlined in CI — the cost is the number of calls, not the per-call work.
How?
One delegated listener per document, event type and phase. Its callback finds the owning element and fires its callbacks:
ownsSelection), and it contains the selection anchor.ownsSelection, instead of testing every subscriber. That is O(DOM depth), not O(blocks). When there is no selection, an editable can still be focused, so it starts from the focused element instead.selectionchangeandensureSelectionSyncmove ontosubscribeOwnedListenertoo, so they ride the same per-element dispatch and stay ordered ahead of the element's other owned listeners, keeping the record synced before they run. This is required: without it, a later block's sync runs after the shared dispatch and input acts on a stale record.Testing Instructions
writing-flow(208),splitting-merging(44),rich-text,editable-root-compat(6).typemetric on codevitals for the drop.Use of AI Tools
Written with the help of Claude Code.