Rich text: check the selection first in ownsSelection - #80548
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. |
|
Size Change: 0 B Total Size: 7.75 MB |
|
Don't have a strong opinion on merge order. |
|
Unfortunately, there are no changes to typing metrics after #80549 was merged - https://codevitals.run/public/WordPress/gutenberg/metrics?metric=type. Let's rebase and try this one as well. |
|
So has nothing to do with layout? 🤔 |
Reorder ownsSelection so the selection-containment check runs before the editing-host checks. Under editableRoot the host is the canvas wrapper, which contains every editable element, so the host checks pass for all of them and only the selection tells them apart. Running the discriminating check first lets a non-owning instance bail before the host checks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrG35JQKJBX1iNof5wtfjN
d1e7e05 to
2fa1316
Compare
|
Flaky tests detected in 2fa1316. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/29969133947
|
What?
Reorders
ownsSelectionso the selection-containment check runs before the editing-host checks.Follow-up to #80549 (merged). That one read the
contentEditableattribute to drop the forced layout, but didn't move the typing metric. This tries the other axis: ordering.Why?
ownsSelectionruns once per editable per keystroke. UndereditableRootthe host is the canvas wrapper, which contains every editable element, so the host checks (contentEditablereads +contains) pass for every instance and don't tell them apart. Only the selection does.With the host checks first, each of the ~N non-owning instances does two
contentEditablereads and anactiveElement.contains( element )walk before reaching the check that eliminates it. Doing the discriminating check first lets them bail sooner.How?
Move the anchor/focus containment check ahead of the host checks. A non-owning instance now returns after one
element.contains( anchorNode ), instead of also doing the twocontentEditablereads and theactiveElement.contains( element )walk.Same boolean, behavior unchanged. This is purely about the order of cheap checks; the forced-layout read is already gone in trunk via #80549.
Testing Instructions
ownsSelectionreturns the same value for the same state.npm run test:unit packages/rich-textpasses.typemetric on codevitals for whether the reordering alone moves it.Use of AI Tools
Written with the help of Claude Code.