diff --git a/packages/block-library/src/table/edit.js b/packages/block-library/src/table/edit.js index c1299ebd820329..2550790ef7680a 100644 --- a/packages/block-library/src/table/edit.js +++ b/packages/block-library/src/table/edit.js @@ -631,6 +631,7 @@ const Cell = memo( function ( { ) } > { diff --git a/packages/core-data/src/awareness/block-lookup.ts b/packages/core-data/src/awareness/block-lookup.ts index 8212758b654377..6c200bedace6a1 100644 --- a/packages/core-data/src/awareness/block-lookup.ts +++ b/packages/core-data/src/awareness/block-lookup.ts @@ -22,6 +22,40 @@ interface EditorStoreBlock { innerBlocks: EditorStoreBlock[]; } +/** + * Find the block Y.Map that contains a nested Yjs type. + * + * Rich-text attributes are often stored directly at attributes.content, but + * blocks can also store rich text deeper inside object or array attributes. + * Walk upward until we find the block map instead of assuming a fixed parent + * depth. + * + * @param yType - The nested Yjs type to start from. + * @return The containing block Y.Map, or null if no block ancestor exists. + */ +export function getContainingBlockYMap( + yType: Y.AbstractType< any > +): Y.Map< unknown > | null { + let current: Y.AbstractType< any > | null = yType; + + while ( current ) { + const parent = current.parent; + + if ( + parent instanceof Y.Map && + parent.parent instanceof Y.Array && + parent.get( 'clientId' ) !== undefined && + parent.get( 'innerBlocks' ) instanceof Y.Array + ) { + return parent; + } + + current = parent instanceof Y.AbstractType ? parent : null; + } + + return null; +} + /** * Given a Y.Map within a Ydoc, traverse up the Yjs block tree to compute the * index path from the root. diff --git a/packages/core-data/src/awareness/post-editor-awareness.ts b/packages/core-data/src/awareness/post-editor-awareness.ts index 9a4edf3b876bb1..b17c606a73f293 100644 --- a/packages/core-data/src/awareness/post-editor-awareness.ts +++ b/packages/core-data/src/awareness/post-editor-awareness.ts @@ -10,7 +10,11 @@ import { store as blockEditorStore } from '@wordpress/block-editor'; * Internal dependencies */ import { BaseAwarenessState, baseEqualityFieldChecks } from './base-awareness'; -import { getBlockPathInYdoc, resolveBlockClientIdByPath } from './block-lookup'; +import { + getBlockPathInYdoc, + getContainingBlockYMap, + resolveBlockClientIdByPath, +} from './block-lookup'; import { AWARENESS_CURSOR_UPDATE_THROTTLE_IN_MS, LOCAL_CURSOR_UPDATE_DEBOUNCE_IN_MS, @@ -27,7 +31,11 @@ import { } from '../utils/crdt-user-selections'; import { SelectionDirection } from '../types'; -import type { SelectionState, WPBlockSelection } from '../types'; +import type { + ResolvedSelection, + SelectionState, + WPBlockSelection, +} from '../types'; import type { YBlocks } from '../utils/crdt-blocks'; import type { DebugCollaboratorData, @@ -239,12 +247,15 @@ export class PostEditorAwareness extends BaseAwarenessState< PostEditorState > { * @param selection - The selection state. * @return The rich-text offset and block client ID, or nulls if not resolvable. */ - public convertSelectionStateToAbsolute( selection: SelectionState ): { - richTextOffset: number | null; - localClientId: string | null; - } { + public convertSelectionStateToAbsolute( + selection: SelectionState + ): ResolvedSelection { if ( selection.type === SelectionType.None ) { - return { richTextOffset: null, localClientId: null }; + return { + richTextOffset: null, + localClientId: null, + attributeKey: null, + }; } if ( selection.type === SelectionType.WholeBlock ) { @@ -267,7 +278,11 @@ export class PostEditorAwareness extends BaseAwarenessState< PostEditorState > { } } - return { richTextOffset: null, localClientId }; + return { + richTextOffset: null, + localClientId, + attributeKey: null, + }; } // Text-based selections: resolve cursor position and navigate up. @@ -282,13 +297,15 @@ export class PostEditorAwareness extends BaseAwarenessState< PostEditorState > { ); if ( ! absolutePosition ) { - return { richTextOffset: null, localClientId: null }; + return { + richTextOffset: null, + localClientId: null, + attributeKey: null, + }; } - // Navigate up: Y.Text -> attributes Y.Map -> block Y.Map - const yType = absolutePosition.type.parent?.parent; - const path = - yType instanceof Y.Map ? getBlockPathInYdoc( yType ) : null; + const yType = getContainingBlockYMap( absolutePosition.type ); + const path = yType ? getBlockPathInYdoc( yType ) : null; const localClientId = path ? resolveBlockClientIdByPath( path ) : null; return { @@ -297,6 +314,7 @@ export class PostEditorAwareness extends BaseAwarenessState< PostEditorState > { asHtmlStringIndex( absolutePosition.index ) ), localClientId, + attributeKey: cursorPos.attributeKey ?? null, }; } diff --git a/packages/core-data/src/awareness/test/block-lookup.ts b/packages/core-data/src/awareness/test/block-lookup.ts index 45e9b031d574ef..2266b67a5918db 100644 --- a/packages/core-data/src/awareness/test/block-lookup.ts +++ b/packages/core-data/src/awareness/test/block-lookup.ts @@ -9,6 +9,7 @@ import { select } from '@wordpress/data'; */ import { getBlockPathInYdoc, + getContainingBlockYMap, resolveBlockClientIdByPath, } from '../block-lookup'; @@ -242,6 +243,75 @@ describe( 'getBlockPathInYdoc', () => { } ); } ); +describe( 'getContainingBlockYMap', () => { + it( 'should find the containing block for direct rich text content', () => { + const block = createTestYBlock( 'block' ); + const attributes = new Y.Map< any >(); + const text = new Y.Text( 'Direct text' ); + attributes.set( 'content', text ); + block.set( 'attributes', attributes ); + + const ydoc = new Y.Doc(); + const rootMap = ydoc.getMap( 'test' ); + const blocks = new Y.Array< Y.Map< any > >(); + rootMap.set( 'blocks', blocks ); + blocks.push( [ block ] ); + + expect( getContainingBlockYMap( text ) ).toBe( block ); + } ); + + it( 'should find the containing block for deeply nested rich text attributes', () => { + const block = createTestYBlock( 'block' ); + const attributes = new Y.Map< any >(); + const cards = new Y.Array< Y.Map< any > >(); + const card = new Y.Map< any >(); + const meta = new Y.Map< any >(); + const caption = new Y.Text( 'Nested caption' ); + + meta.set( 'caption', caption ); + card.set( 'meta', meta ); + cards.push( [ card ] ); + attributes.set( 'cards', cards ); + block.set( 'attributes', attributes ); + + const ydoc = new Y.Doc(); + const rootMap = ydoc.getMap( 'test' ); + const blocks = new Y.Array< Y.Map< any > >(); + rootMap.set( 'blocks', blocks ); + blocks.push( [ block ] ); + + expect( getContainingBlockYMap( caption ) ).toBe( block ); + } ); + + it( 'should return null when no block ancestor exists', () => { + const orphanAttributes = new Y.Map< any >(); + const text = new Y.Text( 'Orphan text' ); + orphanAttributes.set( 'content', text ); + + expect( getContainingBlockYMap( text ) ).toBeNull(); + } ); + + it( 'should skip nested attribute maps that look like blocks', () => { + const block = createTestYBlock( 'block' ); + const attributes = new Y.Map< any >(); + const blockLikeAttribute = new Y.Map< any >(); + const text = new Y.Text( 'Nested text' ); + blockLikeAttribute.set( 'clientId', 'attribute-client-id' ); + blockLikeAttribute.set( 'innerBlocks', new Y.Array() ); + blockLikeAttribute.set( 'content', text ); + attributes.set( 'nested', blockLikeAttribute ); + block.set( 'attributes', attributes ); + + const ydoc = new Y.Doc(); + const rootMap = ydoc.getMap( 'test' ); + const blocks = new Y.Array< Y.Map< any > >(); + rootMap.set( 'blocks', blocks ); + blocks.push( [ block ] ); + + expect( getContainingBlockYMap( text ) ).toBe( block ); + } ); +} ); + describe( 'resolveBlockClientIdByPath', () => { afterEach( () => { jest.restoreAllMocks(); diff --git a/packages/core-data/src/awareness/test/post-editor-awareness.ts b/packages/core-data/src/awareness/test/post-editor-awareness.ts index e093fe489dbc20..3d98ea40768378 100644 --- a/packages/core-data/src/awareness/test/post-editor-awareness.ts +++ b/packages/core-data/src/awareness/test/post-editor-awareness.ts @@ -69,6 +69,65 @@ interface MockBlockEditorOverrides { getSelectionEnd?: jest.Mock; } +type SeededRandom = { + bool: ( probability?: number ) => boolean; + int: ( maxExclusive: number ) => number; + intBetween: ( minInclusive: number, maxInclusive: number ) => number; + pick: < T >( values: readonly T[] ) => T; +}; + +/* eslint-disable no-bitwise */ +function createSeededRandom( seed: number ): SeededRandom { + let state = seed >>> 0; + + if ( state === 0 ) { + state = 0x9e3779b9; + } + + function nextUint32(): number { + state += 0x6d2b79f5; + let value = state; + value = Math.imul( value ^ ( value >>> 15 ), value | 1 ); + value ^= value + Math.imul( value ^ ( value >>> 7 ), value | 61 ); + return ( value ^ ( value >>> 14 ) ) >>> 0; + } + + function next(): number { + return nextUint32() / 0x100000000; + } + + function int( maxExclusive: number ): number { + if ( maxExclusive <= 0 ) { + return 0; + } + + return Math.floor( next() * maxExclusive ); + } + + return { + bool( probability = 0.5 ) { + return next() < probability; + }, + int, + intBetween( minInclusive, maxInclusive ) { + return minInclusive + int( maxInclusive - minInclusive + 1 ); + }, + pick< T >( values: readonly T[] ): T { + if ( values.length === 0 ) { + throw new Error( 'Cannot pick from an empty array.' ); + } + + return values[ int( values.length ) ]; + }, + }; +} +/* eslint-enable no-bitwise */ + +const NESTED_SELECTION_SEEDS = Array.from( + { length: 8 }, + ( _value, index ) => 1401 + index +); + /** * Mock the block-editor store selectors returned by `select( blockEditorStore )`. * @@ -170,6 +229,66 @@ function createTestDocWithBlocks( blocks?: Y.Map< any >[] ) { return ydoc; } +type NestedTextTarget = { + label: string; + text: Y.Text; +}; + +function createNestedAttributeBlock( + clientId: string, + seed: number +): { + block: Y.Map< any >; + targets: NestedTextTarget[]; +} { + const block = new Y.Map(); + block.set( 'clientId', clientId ); + block.set( 'name', 'test/nested-rich-text' ); + + const attrs = new Y.Map(); + const hero = new Y.Map(); + const headline = new Y.Text( `Headline ${ seed } alpha beta` ); + const caption = new Y.Text( `Caption ${ seed } gamma delta` ); + hero.set( 'headline', headline ); + hero.set( 'caption', caption ); + + const cards = new Y.Array(); + const card0 = new Y.Map(); + const card0Title = new Y.Text( `Card ${ seed } title one` ); + const card0Body = new Y.Text( `Card ${ seed } body one two` ); + const card0Meta = new Y.Map(); + const card0Caption = new Y.Text( `Meta ${ seed } caption` ); + card0.set( 'title', card0Title ); + card0.set( 'body', card0Body ); + card0Meta.set( 'caption', card0Caption ); + card0.set( 'meta', card0Meta ); + + const card1 = new Y.Map(); + const card1Title = new Y.Text( `Card ${ seed } title two` ); + const card1Body = new Y.Text( `Card ${ seed } body three four` ); + card1.set( 'title', card1Title ); + card1.set( 'body', card1Body ); + cards.push( [ card0, card1 ] ); + + attrs.set( 'hero', hero ); + attrs.set( 'cards', cards ); + block.set( 'attributes', attrs ); + block.set( 'innerBlocks', new Y.Array() ); + + return { + block, + targets: [ + { label: 'hero.headline', text: headline }, + { label: 'hero.caption', text: caption }, + { label: 'cards.0.title', text: card0Title }, + { label: 'cards.0.body', text: card0Body }, + { label: 'cards.0.meta.caption', text: card0Caption }, + { label: 'cards.1.title', text: card1Title }, + { label: 'cards.1.body', text: card1Body }, + ], + }; +} + describe( 'PostEditorAwareness', () => { let doc: Y.Doc; let subscribeCallback: ( () => void ) | null = null; @@ -528,6 +647,7 @@ describe( 'PostEditorAwareness', () => { cursorPosition: { relativePosition, absoluteOffset: 5, + attributeKey: 'content', }, }; @@ -536,6 +656,7 @@ describe( 'PostEditorAwareness', () => { expect( result.richTextOffset ).toBe( 5 ); expect( result.localClientId ).toBe( 'block-1' ); + expect( result.attributeKey ).toBe( 'content' ); } ); test( 'should resolve WholeBlock selection to block client ID', () => { @@ -568,6 +689,58 @@ describe( 'PostEditorAwareness', () => { expect( result.richTextOffset ).toBeNull(); expect( result.localClientId ).toBe( 'block-1' ); + expect( result.attributeKey ).toBeNull(); + } ); + + test( 'should return null attributeKey for SelectionType.None', () => { + const awareness = new PostEditorAwareness( + doc, + 'postType', + 'post', + 123 + ); + + const result = awareness.convertSelectionStateToAbsolute( { + type: SelectionType.None, + } ); + + expect( result.attributeKey ).toBeNull(); + } ); + + test( 'should pass through nested attributeKey for a cursor selection', () => { + const awareness = new PostEditorAwareness( + doc, + 'postType', + 'post', + 123 + ); + + const documentMap = doc.getMap( CRDT_RECORD_MAP_KEY ); + const blocks = documentMap.get( 'blocks' ) as Y.Array< + Y.Map< any > + >; + const block = blocks.get( 0 ); + const attrs = block.get( 'attributes' ) as Y.Map< Y.Text >; + const yText = attrs.get( 'content' ); + + const relativePosition = Y.createRelativePositionFromTypeIndex( + yText as Y.Text, + 3 + ); + + const selection: SelectionCursor = { + type: SelectionType.Cursor, + cursorPosition: { + relativePosition, + absoluteOffset: 3, + attributeKey: 'body.0.cells.0.content', + }, + }; + + const result = + awareness.convertSelectionStateToAbsolute( selection ); + + expect( result.attributeKey ).toBe( 'body.0.cells.0.content' ); } ); } ); @@ -1001,6 +1174,76 @@ describe( 'PostEditorAwareness', () => { } ); } ); + describe( 'convertSelectionStateToAbsolute with nested rich-text attributes', () => { + test.each( NESTED_SELECTION_SEEDS )( + 'resolves fuzzed nested rich-text cursor (seed %i)', + ( seed ) => { + const rng = createSeededRandom( seed ); + const { block, targets } = createNestedAttributeBlock( + 'yjs-nested-attrs', + seed + ); + const nestedDoc = createTestDocWithBlocks( [ block ] ); + + mockBlockEditorStore( { + blocks: [ + { + clientId: 'local-nested-attrs', + innerBlocks: [], + }, + ], + } ); + + const target = rng.pick( targets ); + const initialOffset = rng.intBetween( + 1, + Math.max( 1, target.text.length - 1 ) + ); + const relativePosition = Y.createRelativePositionFromTypeIndex( + target.text, + initialOffset + ); + let expectedOffset = initialOffset; + + if ( rng.bool() ) { + const prefix = `p${ seed % 97 } `; + target.text.insert( 0, prefix ); + expectedOffset += prefix.length; + } else { + const deleteLength = Math.min( + initialOffset, + rng.intBetween( 1, 3 ) + ); + target.text.delete( 0, deleteLength ); + expectedOffset -= deleteLength; + } + + const awareness = new PostEditorAwareness( + nestedDoc, + 'postType', + 'post', + 123 + ); + + const selection: SelectionCursor = { + type: SelectionType.Cursor, + cursorPosition: { + relativePosition, + absoluteOffset: initialOffset, + }, + }; + + const result = + awareness.convertSelectionStateToAbsolute( selection ); + + expect( result.richTextOffset ).toBe( expectedOffset ); + expect( result.localClientId ).toBe( 'local-nested-attrs' ); + + nestedDoc.destroy(); + } + ); + } ); + describe( 'template mode (core/post-content handling)', () => { test( 'should resolve cursor when getBlocks returns template tree with core/post-content', () => { // Yjs doc has only the post content blocks (no template wrapper) diff --git a/packages/core-data/src/hooks/test/use-post-editor-awareness-state.ts b/packages/core-data/src/hooks/test/use-post-editor-awareness-state.ts index 065f0cefe72b35..c1c49796252684 100644 --- a/packages/core-data/src/hooks/test/use-post-editor-awareness-state.ts +++ b/packages/core-data/src/hooks/test/use-post-editor-awareness-state.ts @@ -295,6 +295,7 @@ describe( 'use-post-editor-awareness-state hooks', () => { expect( result.current( mockSelection ) ).toEqual( { richTextOffset: null, localClientId: null, + attributeKey: null, } ); } ); @@ -309,6 +310,7 @@ describe( 'use-post-editor-awareness-state hooks', () => { mockAwareness.convertSelectionStateToAbsolute.mockReturnValue( { richTextOffset: 10, localClientId: 'block-1', + attributeKey: 'content', } ); const { result } = renderHook( () => @@ -323,6 +325,7 @@ describe( 'use-post-editor-awareness-state hooks', () => { expect( position ).toEqual( { richTextOffset: 10, localClientId: 'block-1', + attributeKey: 'content', } ); } ); } ); diff --git a/packages/core-data/src/hooks/use-post-editor-awareness-state.ts b/packages/core-data/src/hooks/use-post-editor-awareness-state.ts index 19eb10dc863d81..e4b2b7090f5a08 100644 --- a/packages/core-data/src/hooks/use-post-editor-awareness-state.ts +++ b/packages/core-data/src/hooks/use-post-editor-awareness-state.ts @@ -27,6 +27,7 @@ interface AwarenessState { const defaultResolvedSelection: ResolvedSelection = { richTextOffset: null, localClientId: null, + attributeKey: null, }; const defaultState: AwarenessState = { diff --git a/packages/core-data/src/types.ts b/packages/core-data/src/types.ts index c3b506fac98d93..8a79320fa5e630 100644 --- a/packages/core-data/src/types.ts +++ b/packages/core-data/src/types.ts @@ -127,6 +127,10 @@ export type CursorPosition = { // character. With both of these values as editor state, a change in perceived // position will always result in a redraw. absoluteOffset: number; + + // The sender's `WPBlockSelection.attributeKey` (e.g. `content` or + // `body.0.cells.0.content`). + attributeKey?: string; }; /** @@ -192,4 +196,12 @@ export type SelectionState = export interface ResolvedSelection { richTextOffset: number | null; localClientId: string | null; + + // Identifier of the RichText attribute within the block, e.g.: + // - `content` on a core/paragraph block + // - `citation` on a quote block + // - a dot path into a nested attribute like `body.0.cells.0.content` for a + // core/table cell. + // Set to `null` for WholeBlock selections. + attributeKey: string | null; } diff --git a/packages/core-data/src/utils/block-selection-history.ts b/packages/core-data/src/utils/block-selection-history.ts index 963f3fb264431c..5bb373e9f5f9b6 100644 --- a/packages/core-data/src/utils/block-selection-history.ts +++ b/packages/core-data/src/utils/block-selection-history.ts @@ -12,6 +12,7 @@ import { Y } from '@wordpress/sync'; import { asRichTextOffset, findBlockByClientIdInDoc, + getYTextByAttributeKey, richTextOffsetToHtmlIndex, } from './crdt-utils'; import type { WPBlockSelection, WPSelection } from '../types'; @@ -147,14 +148,16 @@ function convertWPBlockSelectionToSelection( const attributes = block?.get( 'attributes' ); const attributeKey = selection.attributeKey; - const changedYText = attributeKey - ? attributes?.get( attributeKey ) - : undefined; - - const isYText = changedYText instanceof Y.Text; - const isFullyDefinedSelection = attributeKey && clientId; + let changedYText: Y.Text | null = null; + if ( attributeKey && attributes ) { + changedYText = getYTextByAttributeKey( attributes, attributeKey ); + } - if ( ! isYText || ! isFullyDefinedSelection ) { + if ( + ! ( changedYText instanceof Y.Text ) || + ! attributeKey || + ! clientId + ) { // We either don't have a valid YText (it's been deleted) or we've // been passed a selection that's just a block clientId. // Store as BlockSelection. diff --git a/packages/core-data/src/utils/crdt-user-selections.ts b/packages/core-data/src/utils/crdt-user-selections.ts index deea6e7d2a735c..04c227eb20a76b 100644 --- a/packages/core-data/src/utils/crdt-user-selections.ts +++ b/packages/core-data/src/utils/crdt-user-selections.ts @@ -15,6 +15,7 @@ import type { YBlock, YBlocks } from './crdt-blocks'; import { asRichTextOffset, getRootMap, + getYTextByAttributeKey, richTextOffsetToHtmlIndex, } from './crdt-utils'; import type { @@ -173,7 +174,9 @@ function getCursorPosition( } const attributes = block.get( 'attributes' ); - const currentYText = attributes?.get( selection.attributeKey ); + const currentYText = attributes + ? getYTextByAttributeKey( attributes, selection.attributeKey ) + : null; // If the attribute is not a Y.Text, return null. if ( ! ( currentYText instanceof Y.Text ) ) { @@ -191,6 +194,7 @@ function getCursorPosition( return { relativePosition, absoluteOffset: selection.offset, + attributeKey: selection.attributeKey, }; } diff --git a/packages/core-data/src/utils/crdt-utils.ts b/packages/core-data/src/utils/crdt-utils.ts index f37b01cfac0d2a..ad9cbc03c10d84 100644 --- a/packages/core-data/src/utils/crdt-utils.ts +++ b/packages/core-data/src/utils/crdt-utils.ts @@ -125,6 +125,47 @@ export function asHtmlStringIndex( index: number ): HtmlStringIndex { return index as HtmlStringIndex; } +/** + * Resolve a selection attribute key to a Y.Text value. + * + * RichText identifiers are normally top-level block attribute keys, but nested + * rich-text fields can provide a dot path such as `body.0.cells.0.content`. + * + * @param attributes The block attributes map. + * @param attributeKey The top-level attribute key or nested attribute path. + * @return The matching Y.Text, or null if the path is not a rich-text field. + */ +export function getYTextByAttributeKey( + attributes: Y.Map< unknown >, + attributeKey: string +): Y.Text | null { + const directValue = attributes.get( attributeKey ); + if ( directValue instanceof Y.Text ) { + return directValue; + } + + let value: unknown = attributes; + for ( const pathPart of attributeKey.split( '.' ) ) { + if ( value instanceof Y.Map ) { + value = value.get( pathPart ); + } else if ( value instanceof Y.Array ) { + const index = Number.parseInt( pathPart, 10 ); + if ( + ! Number.isSafeInteger( index ) || + index < 0 || + index.toString() !== pathPart + ) { + return null; + } + value = value.get( index ); + } else { + return null; + } + } + + return value instanceof Y.Text ? value : null; +} + /** * Given a block ID and a Y.Doc, find the block in the document. * diff --git a/packages/core-data/src/utils/test/block-selection-history.test.ts b/packages/core-data/src/utils/test/block-selection-history.test.ts index 8a88b5a7d45717..0b18d3af2c6f7f 100644 --- a/packages/core-data/src/utils/test/block-selection-history.test.ts +++ b/packages/core-data/src/utils/test/block-selection-history.test.ts @@ -38,6 +38,15 @@ function createTestDoc() { block2.set( 'clientId', 'block-2' ); const block2Attrs = new Y.Map(); block2Attrs.set( 'content', new Y.Text( 'Second block' ) ); + const body = new Y.Array(); + const row = new Y.Map(); + const cells = new Y.Array(); + const cell = new Y.Map(); + cell.set( 'content', new Y.Text( 'Cell text' ) ); + cells.push( [ cell ] ); + row.set( 'cells', cells ); + body.push( [ row ] ); + block2Attrs.set( 'body', body ); block2.set( 'attributes', block2Attrs ); block2.set( 'innerBlocks', new Y.Array() ); blocks.push( [ block2 ] ); @@ -210,6 +219,39 @@ describe( 'BlockSelectionHistory', () => { const endPosition = fullSelection.end as YRelativeSelection; expect( endPosition.offset ).toBe( 0 ); } ); + + test( 'should convert nested rich-text attribute paths to relative positions', () => { + const selection = createSelection( { + clientId: 'block-2', + attributeKey: 'body.0.cells.0.content', + offset: 4, + } ); + + history.updateSelection( selection ); + + const selectionHistory = history.getSelectionHistory(); + expect( selectionHistory.length ).toBe( 1 ); + + const fullSelection = selectionHistory[ 0 ]; + expect( fullSelection.start.type ).toBe( + YSelectionType.RelativeSelection + ); + expect( fullSelection.end.type ).toBe( + YSelectionType.RelativeSelection + ); + + const startPosition = fullSelection.start as YRelativeSelection; + const endPosition = fullSelection.end as YRelativeSelection; + + expect( startPosition.attributeKey ).toBe( + 'body.0.cells.0.content' + ); + expect( startPosition.offset ).toBe( 4 ); + expect( startPosition.relativePosition ).toBeDefined(); + expect( endPosition.attributeKey ).toBe( 'body.0.cells.0.content' ); + expect( endPosition.offset ).toBe( 4 ); + expect( endPosition.relativePosition ).toBeDefined(); + } ); } ); describe( 'updateSelection with block positions', () => { diff --git a/packages/core-data/src/utils/test/crdt-user-selections.ts b/packages/core-data/src/utils/test/crdt-user-selections.ts index b3b36bf2140ebf..5c9a526cc4c3f5 100644 --- a/packages/core-data/src/utils/test/crdt-user-selections.ts +++ b/packages/core-data/src/utils/test/crdt-user-selections.ts @@ -396,6 +396,15 @@ function createTestDocWithBlocks() { block2.set( 'clientId', 'block-2' ); const block2Attrs = new Y.Map(); block2Attrs.set( 'content', new Y.Text( 'Second block content' ) ); + const body = new Y.Array(); + const row = new Y.Map(); + const cells = new Y.Array(); + const cell = new Y.Map(); + cell.set( 'content', new Y.Text( 'Cell text' ) ); + cells.push( [ cell ] ); + row.set( 'cells', cells ); + body.push( [ row ] ); + block2Attrs.set( 'body', body ); block2.set( 'attributes', block2Attrs ); block2.set( 'innerBlocks', new Y.Array() ); blocks.push( [ block2 ] ); @@ -529,6 +538,9 @@ describe( 'getSelectionState', () => { expect( ( result as SelectionCursor ).cursorPosition.absoluteOffset ).toBe( 5 ); + expect( + ( result as SelectionCursor ).cursorPosition.attributeKey + ).toBe( 'content' ); } ); test( 'returns Cursor at start of block (offset 0)', () => { @@ -555,6 +567,33 @@ describe( 'getSelectionState', () => { ).toBe( 0 ); } ); + test( 'returns Cursor for a nested rich-text attribute path', () => { + const selectionStart: WPBlockSelection = { + clientId: 'block-2', + attributeKey: 'body.0.cells.0.content', + offset: 4, + }; + const selectionEnd: WPBlockSelection = { + clientId: 'block-2', + attributeKey: 'body.0.cells.0.content', + offset: 4, + }; + + const result = getSelectionState( + selectionStart, + selectionEnd, + testDoc + ); + + expect( result.type ).toBe( SelectionType.Cursor ); + expect( + ( result as SelectionCursor ).cursorPosition.absoluteOffset + ).toBe( 4 ); + expect( + ( result as SelectionCursor ).cursorPosition.attributeKey + ).toBe( 'body.0.cells.0.content' ); + } ); + test( 'returns None when block does not exist', () => { const selectionStart: WPBlockSelection = { clientId: 'non-existent-block', diff --git a/packages/core-data/src/utils/test/crdt-utils.ts b/packages/core-data/src/utils/test/crdt-utils.ts index 3bdb69b41514cb..16333318deec4d 100644 --- a/packages/core-data/src/utils/test/crdt-utils.ts +++ b/packages/core-data/src/utils/test/crdt-utils.ts @@ -2,6 +2,7 @@ * External dependencies */ import { describe, expect, it } from '@jest/globals'; +import { Y } from '@wordpress/sync'; /** * Internal dependencies @@ -9,6 +10,7 @@ import { describe, expect, it } from '@jest/globals'; import { asHtmlStringIndex, asRichTextOffset, + getYTextByAttributeKey, htmlIndexToRichTextOffset as typedHtmlIndexToRichTextOffset, richTextOffsetToHtmlIndex as typedRichTextOffsetToHtmlIndex, } from '../crdt-utils'; @@ -27,6 +29,56 @@ function richTextOffsetToHtmlIndex( html: string, richTextOffset: number ) { ); } +function createAttachedAttributes(): Y.Map< unknown > { + const ydoc = new Y.Doc(); + const root = ydoc.getMap( 'test' ); + const attributes = new Y.Map< unknown >(); + root.set( 'attributes', attributes ); + return attributes; +} + +describe( 'getYTextByAttributeKey', () => { + it( 'returns a top-level rich-text attribute', () => { + const attributes = createAttachedAttributes(); + const text = new Y.Text( 'Top level' ); + attributes.set( 'content', text ); + + expect( getYTextByAttributeKey( attributes, 'content' ) ).toBe( text ); + } ); + + it( 'returns a nested rich-text attribute by dot path', () => { + const attributes = createAttachedAttributes(); + const body = new Y.Array< Y.Map< unknown > >(); + const row = new Y.Map< unknown >(); + const cells = new Y.Array< Y.Map< unknown > >(); + const cell = new Y.Map< unknown >(); + const text = new Y.Text( 'Cell text' ); + + cell.set( 'content', text ); + cells.push( [ cell ] ); + row.set( 'cells', cells ); + body.push( [ row ] ); + attributes.set( 'body', body ); + + expect( + getYTextByAttributeKey( attributes, 'body.0.cells.0.content' ) + ).toBe( text ); + } ); + + it( 'returns null for invalid array path segments', () => { + const attributes = createAttachedAttributes(); + const body = new Y.Array< Y.Map< unknown > >(); + attributes.set( 'body', body ); + + expect( + getYTextByAttributeKey( attributes, 'body.01.cells.0.content' ) + ).toBeNull(); + expect( + getYTextByAttributeKey( attributes, 'body.-1.cells.0.content' ) + ).toBeNull(); + } ); +} ); + describe( 'htmlIndexToRichTextOffset', () => { it( 'returns the index unchanged when there are no tags', () => { expect( htmlIndexToRichTextOffset( 'hello world', 5 ) ).toBe( 5 ); diff --git a/packages/editor/src/components/collaborators-overlay/compute-selection.ts b/packages/editor/src/components/collaborators-overlay/compute-selection.ts index f0958a85c5ab72..43787331a7d77a 100644 --- a/packages/editor/src/components/collaborators-overlay/compute-selection.ts +++ b/packages/editor/src/components/collaborators-overlay/compute-selection.ts @@ -36,6 +36,44 @@ export interface SelectionVisual { selectionRects?: SelectionRect[]; } +/** + * Resolve the most specific editor element the selection refers to. + * + * When the sender carries an `attributeKey`, narrow to the RichText element + * matching `data-wp-block-attribute-key` inside the block. This is what makes + * cursor placement work for blocks with multiple RichText fields (e.g. + * `core/table` cells: `body.0.cells.0.content`, etc.). Falls back to the + * block element when `attributeKey` is missing (WholeBlock selections, + * older senders, or DOM lookup miss). + * + * @param editorDocument - The editor document. + * @param resolvedSelection - The resolved selection. + * @return The target element (RichText editable or block), or null. + */ +function resolveTargetElement( + editorDocument: Document, + resolvedSelection: ResolvedSelection +): HTMLElement | null { + if ( ! resolvedSelection.localClientId ) { + return null; + } + + const blockElement = editorDocument.querySelector< HTMLElement >( + `[data-block="${ resolvedSelection.localClientId }"]` + ); + + if ( ! blockElement || ! resolvedSelection.attributeKey ) { + return blockElement; + } + + const attrKey = CSS.escape( resolvedSelection.attributeKey ); + return ( + blockElement.querySelector< HTMLElement >( + `[data-wp-block-attribute-key="${ attrKey }"]` + ) ?? blockElement + ); +} + /** * Compute cursor coords and optional selection rects for a single user's selection. * @@ -83,14 +121,14 @@ function computeCursorOnly( if ( ! start.localClientId ) { return {}; } - const blockElement = - overlayContext.editorDocument.querySelector< HTMLElement >( - `[data-block="${ start.localClientId }"]` - ); + const targetElement = resolveTargetElement( + overlayContext.editorDocument, + start + ); return { coords: getCursorPosition( start.richTextOffset, - blockElement, + targetElement, overlayContext.editorDocument, overlayContext.overlayRect ), @@ -157,10 +195,10 @@ function computeTextSelection( } // Fallback: cursor at start position only. - const startBlock = - overlayContext.editorDocument.querySelector< HTMLElement >( - `[data-block="${ start.localClientId }"]` - ); + const startBlock = resolveTargetElement( + overlayContext.editorDocument, + start + ); return { coords: getCursorPosition( @@ -185,10 +223,10 @@ function computeSingleBlockRects( end: ResolvedSelection, overlayContext: OverlayContext ): SingleBlockResult { - const blockElement = - overlayContext.editorDocument.querySelector< HTMLElement >( - `[data-block="${ start.localClientId }"]` - ); + const blockElement = resolveTargetElement( + overlayContext.editorDocument, + start + ); if ( ! blockElement || start.richTextOffset === null || @@ -227,11 +265,13 @@ function computeMultiBlockRects( ): MultiBlockResult { let docFirst = start; let docLast = end; - let firstBlock = overlayContext.editorDocument.querySelector< HTMLElement >( - `[data-block="${ docFirst.localClientId }"]` + let firstBlock = resolveTargetElement( + overlayContext.editorDocument, + docFirst ); - let lastBlock = overlayContext.editorDocument.querySelector< HTMLElement >( - `[data-block="${ docLast.localClientId }"]` + let lastBlock = resolveTargetElement( + overlayContext.editorDocument, + docLast ); // Swap to document order if needed. diff --git a/packages/editor/src/components/collaborators-overlay/use-render-cursors.ts b/packages/editor/src/components/collaborators-overlay/use-render-cursors.ts index 61005ca094672d..0d77542387c6ad 100644 --- a/packages/editor/src/components/collaborators-overlay/use-render-cursors.ts +++ b/packages/editor/src/components/collaborators-overlay/use-render-cursors.ts @@ -106,6 +106,7 @@ export function useRenderCursors( let start: ResolvedSelection = { richTextOffset: null, localClientId: null, + attributeKey: null, }; let end: ResolvedSelection | undefined; diff --git a/test/e2e/specs/editor/collaboration/collaboration-code-editor-performance.spec.ts b/test/e2e/specs/editor/collaboration/collaboration-code-editor-performance.spec.ts index 0f48429f552168..365b43170b7725 100644 --- a/test/e2e/specs/editor/collaboration/collaboration-code-editor-performance.spec.ts +++ b/test/e2e/specs/editor/collaboration/collaboration-code-editor-performance.spec.ts @@ -38,8 +38,11 @@ const test = base.extend< Fixtures >( { collaborationEnabled: [ async ( { requestUtils }, use ) => { await setCollaboration( requestUtils, true ); - await use( true ); - await setCollaboration( requestUtils, false ); + try { + await use( true ); + } finally { + await setCollaboration( requestUtils, false ); + } }, { auto: true }, ], diff --git a/test/e2e/specs/editor/collaboration/collaboration-nested-awareness-selection.spec.ts b/test/e2e/specs/editor/collaboration/collaboration-nested-awareness-selection.spec.ts new file mode 100644 index 00000000000000..2ead9c2e10b74d --- /dev/null +++ b/test/e2e/specs/editor/collaboration/collaboration-nested-awareness-selection.spec.ts @@ -0,0 +1,105 @@ +/** + * Internal dependencies + */ +import { test, expect } from './fixtures'; + +test.describe( 'Collaboration - Nested Awareness Selection', () => { + test( 'cursor in a table cell appears in the same cell for another user', async ( { + collaborationUtils, + requestUtils, + editor, + page, + } ) => { + const post = await requestUtils.createPost( { + title: 'Nested Awareness Selection Test', + status: 'draft', + date_gmt: new Date().toISOString(), + content: + '\n' + + '
' + + '' + + '' + + '
AlphaBeta
GammaDelta
\n' + + '', + } ); + + await collaborationUtils.openCollaborativeSession( post.id ); + + const { page2 } = collaborationUtils; + + await expect + .poll( () => collaborationUtils.editor2.getBlocks(), { + timeout: 10000, + } ) + .toMatchObject( [ + { + name: 'core/table', + }, + ] ); + + // Target the last cell — row 1, column 1 ("Delta"), which is nth=3 + // in a 2x2 grid. Picking the last cell maximizes distance from the + // first cell, so a bug that flattens offsets onto cell (0,0) is + // easy to detect via bounding boxes below. + const targetCell = editor.canvas.locator( + 'role=textbox[name="Body cell text"i] >> nth=3' + ); + + await targetCell.click(); + await targetCell.click(); + await page.keyboard.press( 'End' ); + + // Sender side: selection state should point to the target cell's + // nested attribute path. + await expect + .poll( + () => + page.evaluate( + () => + window.wp.data + .select( 'core/block-editor' ) + .getSelectionStart()?.attributeKey ?? '' + ), + { timeout: 5000 } + ) + .toBe( 'body.1.cells.1.content' ); + + const editorFrame = page2.frameLocator( + 'iframe[name="editor-canvas"]' + ); + const cursor = editorFrame.locator( + '.collaborators-overlay-user-cursor' + ); + + await expect + .poll( () => cursor.count(), { timeout: 15000 } ) + .toBeGreaterThan( 0 ); + + const cursorBox = await cursor.first().boundingBox(); + if ( ! cursorBox ) { + throw new Error( 'Collaborator cursor bounding box not available' ); + } + expect( cursorBox.height ).toBeGreaterThan( 0 ); + + // Receiver side: verify the rendered cursor lands inside the same + // cell, not flattened onto the first cell. + const remoteCell = editorFrame.locator( + 'role=textbox[name="Body cell text"i] >> nth=3' + ); + const cellBox = await remoteCell.boundingBox(); + if ( ! cellBox ) { + throw new Error( 'Remote target cell bounding box not available' ); + } + + const cursorCenterX = cursorBox.x + cursorBox.width / 2; + const cursorCenterY = cursorBox.y + cursorBox.height / 2; + expect( cursorCenterX ).toBeGreaterThanOrEqual( cellBox.x ); + expect( cursorCenterX ).toBeLessThanOrEqual( + cellBox.x + cellBox.width + ); + expect( cursorCenterY ).toBeGreaterThanOrEqual( cellBox.y ); + expect( cursorCenterY ).toBeLessThanOrEqual( + cellBox.y + cellBox.height + ); + } ); +} ); diff --git a/test/e2e/specs/editor/collaboration/collaboration-refresh.spec.ts b/test/e2e/specs/editor/collaboration/collaboration-refresh.spec.ts index 0cf4fa86bbd5a6..fd1d4b112626c2 100644 --- a/test/e2e/specs/editor/collaboration/collaboration-refresh.spec.ts +++ b/test/e2e/specs/editor/collaboration/collaboration-refresh.spec.ts @@ -1,41 +1,16 @@ -/** - * External dependencies - */ -import type { BrowserContext, Page } from '@playwright/test'; - -/** - * WordPress dependencies - */ -import { Editor } from '@wordpress/e2e-test-utils-playwright'; - /** * Internal dependencies */ import { test, expect } from './fixtures'; import { SECOND_USER } from './fixtures/collaboration-utils'; -const BASE_URL = process.env.WP_BASE_URL || 'http://localhost:8889'; - test.describe( 'Collaboration - Refresh', () => { - let secondContext: BrowserContext; - let page2: Page; - let editor2: Editor; - - test.afterEach( async () => { - await secondContext?.close(); - } ); - test( 'User A edits are synced to User B after User A refreshes', async ( { collaborationUtils, requestUtils, editor, page, - admin, } ) => { - // Destructuring collaborationUtils activates the fixture which - // enables the collaboration setting and creates the second user. - void collaborationUtils; - const post = await requestUtils.createPost( { title: 'Refresh Sync Test', status: 'draft', @@ -43,15 +18,7 @@ test.describe( 'Collaboration - Refresh', () => { } ); // Step 1: User A opens the post, adds content, and saves. - await admin.editPost( post.id ); - await page.waitForFunction( - () => - ( window as any )._wpCollaborationEnabled === true && - window?.wp?.data && - window?.wp?.blocks, - undefined, - { timeout: 15000 } - ); + await collaborationUtils.openPost( post.id ); await editor.canvas .getByRole( 'button', { name: 'Add default block' } ) @@ -61,49 +28,11 @@ test.describe( 'Collaboration - Refresh', () => { await editor.saveDraft(); // Step 2: User B loads the post and adds content. - secondContext = await page - .context() - .browser()! - .newContext( { baseURL: BASE_URL } ); - page2 = await secondContext.newPage(); - - await page2.goto( '/wp-login.php' ); - await page2.locator( '#user_login' ).fill( SECOND_USER.username ); - await page2.locator( '#user_pass' ).fill( SECOND_USER.password ); - await page2.getByRole( 'button', { name: 'Log In' } ).click(); - await page2.waitForURL( '**/wp-admin/**' ); - - await page2.goto( `/wp-admin/post.php?post=${ post.id }&action=edit` ); - await page2.waitForFunction( - () => window?.wp?.data && window?.wp?.blocks - ); - await page2.evaluate( () => { - window.wp.data - .dispatch( 'core/preferences' ) - .set( 'core/edit-post', 'welcomeGuide', false ); - window.wp.data - .dispatch( 'core/preferences' ) - .set( 'core/edit-post', 'fullscreenMode', false ); - } ); - await page2.waitForFunction( - () => - ( window as any )._wpCollaborationEnabled === true && - window?.wp?.data && - window?.wp?.blocks, - undefined, - { timeout: 15000 } - ); - editor2 = new Editor( { page: page2 } ); + const { page: page2, editor: editor2 } = + await collaborationUtils.joinUser( post.id, SECOND_USER ); // Wait for both users to discover each other via awareness. - await Promise.all( [ - page - .getByRole( 'button', { name: /Collaborators list/ } ) - .waitFor( { timeout: 15000 } ), - page2 - .getByRole( 'button', { name: /Collaborators list/ } ) - .waitFor( { timeout: 15000 } ), - ] ); + await collaborationUtils.waitForMutualDiscovery( { timeout: 30000 } ); // User B adds content below the existing paragraph. await editor2.canvas @@ -132,24 +61,10 @@ test.describe( 'Collaboration - Refresh', () => { await page.reload( { waitUntil: 'load' } ); // Wait for collaboration to re-initialize after refresh. - await page.waitForFunction( - () => - ( window as any )._wpCollaborationEnabled === true && - window?.wp?.data && - window?.wp?.blocks, - undefined, - { timeout: 15000 } - ); + await collaborationUtils.waitForCollaborationReady( page ); // Wait for both users to re-discover each other via awareness. - await Promise.all( [ - page - .getByRole( 'button', { name: /Collaborators list/ } ) - .waitFor( { timeout: 15000 } ), - page2 - .getByRole( 'button', { name: /Collaborators list/ } ) - .waitFor( { timeout: 15000 } ), - ] ); + await collaborationUtils.waitForMutualDiscovery( { timeout: 30000 } ); // Step 4: User A adds new content after refresh. await editor.canvas diff --git a/test/e2e/specs/editor/collaboration/fixtures/index.ts b/test/e2e/specs/editor/collaboration/fixtures/index.ts index d73f61b5fd8cf2..237203889b2255 100644 --- a/test/e2e/specs/editor/collaboration/fixtures/index.ts +++ b/test/e2e/specs/editor/collaboration/fixtures/index.ts @@ -31,8 +31,14 @@ export const test = base.extend< Fixtures >( { await requestUtils.deleteAllUsers(); await requestUtils.createUser( SECOND_USER ); await setCollaboration( requestUtils, true ); - await use( utils ); - await utils.teardown(); - await setCollaboration( requestUtils, false ); + try { + await use( utils ); + } finally { + try { + await utils.teardown(); + } finally { + await setCollaboration( requestUtils, false ); + } + } }, } );