Rich text: Share window listeners across RichText instances#78310
Open
ellatrix wants to merge 3 commits into
Open
Rich text: Share window listeners across RichText instances#78310ellatrix wants to merge 3 commits into
ellatrix wants to merge 3 commits into
Conversation
|
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. |
a5234d3 to
889189c
Compare
|
Size Change: +187 B (0%) Total Size: 7.97 MB 📦 View Changed
ℹ️ View Unchanged
|
Each RichText hook setup adds four `addEventListener` calls to the shared `defaultView` (`copy` / `cut` from copy-handler, `pointerdown` / `pointerup` from prevent-focus-capture). Every handler short-circuits when the rich-text isn't the focused one, so N listeners do the work of 1 — the rest are pure JS↔C++ boundary crossings on mount. With many rich-text instances mounted at once (paragraphs, headings, buttons, lists in a large post each contain a RichText), the boundary-crossing cost is the largest remaining source of `addEventListener` time during the React commit — ~50 ms total in a representative post-editor first-block trace, of which ~10 ms is the shared-target subset this change addresses. Introduce `subscribeSharedListener( target, eventType, callback )` in `packages/rich-text/src/hook/event-listeners/shared-listener.js`: keeps a `WeakMap<EventTarget, Map<eventType, Set<callback>>>` and attaches one native listener per (target, type) that fans out to every subscriber. Refactor `copy-handler` and `prevent-focus-capture` to use it. Same handler semantics, same call order, no API change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…handlers Both `enter` (block-editor/rich-text/event-listeners/enter.js) and `paste-handler` (same dir) attach their listeners to `defaultView` so parent elements can intercept first. Each handler then checks `event.target === element` (or `element.contains(event.target)`) and short-circuits if the rich-text isn't the focused one — same pattern as `copy-handler` and `prevent-focus-capture`. Route both through the shared registry so all RichText instances share one native `keydown` / `paste` listener on the window instead of one each. Expose `subscribeSharedListener` from `@wordpress/rich-text`'s private APIs so block-editor can use the same helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
889189c to
c0de9b8
Compare
Move `delete.js`'s `keydown` listener off the contenteditable and onto `ownerDocument` via `subscribeSharedListener`. The handler now bails if our editable isn't the focused element, mirroring the `copy-handler` / `prevent-focus-capture` pattern. Behaviour for Backspace/Delete-of-all-selected-content is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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?
Make
copy-handlerandprevent-focus-captureregister their window-level listeners through a shared registry instead of each RichText callingdefaultView.addEventListenerdirectly. Same handler semantics; one native listener per (target, event type) instead of N.Why?
Each RichText adds four
addEventListenercalls to the shareddefaultView(copy/cut,pointerdown/pointerup). Every handler short-circuits when the rich-text isn't the focused one — N listeners do the work of 1, the rest are pure JS↔C++ boundary crossings on mount. With many RichTexts mounted at once (paragraphs, headings, buttons, list items each have one), that's the largest remainingaddEventListenersource after #78297: ~50 ms total in a representative post-editor first-block trace, of which the window-targeted subset is ~10–15 ms.How?
Add
subscribeSharedListener( target, eventType, callback )inpackages/rich-text/src/hook/event-listeners/shared-listener.js. It keeps aWeakMap<EventTarget, Map<eventType, Set<callback>>>and attaches one native listener per (target, type) that fans out to every subscriber. Refactorcopy-handlerandprevent-focus-captureto use it.Document-level listeners inside
input-and-selection.jsandselection-change-compat.jsare attached conditionally on user interaction (focus, composition end, pointerdown/keydown) — not mount-time — so they're already not on the cold-mount path and left as-is.Testing
npm run test:unit -- packages/rich-text(213 pass).copy-handlerpath). Confirm clipboard contains expected text/HTML.prevent-focus-capturepath).Use of AI Tools
Authored with Claude Code assistance; reviewed and verified by the author.