Skip to content

Commit 54af1ce

Browse files
danluudanluudmsnellpeterwilsonccalecgeatches
authored
RTC: Ensure that changes are only applied to text and cursors for their associated RichText instances.
When synchronizing realtime collaborative edit sessions, updates have been applying based on attribute name and cursor position, but the origin block and attribute being edited was not compared against the cursor position; the result is that updates could be applied to the wrong block attributes and the cursor position could be incorrectly adjusted as updates roll in. Worse, because of this mismatch, some diffs were being corrupted via bugs in `Delta.diffWithCursor()`. This patch tracks the cursor and attribute origin through the synchronization code to ensure that only the appropriate edits are applied to their corresponding block attributes. A work-around is provided for the `diffWithCursor()` bugs due to the scope of this PR vs. tactically fixing that function: if a trial application of the transformed diff fails to produce the expect output, then the transformed diff is discarded and the original diff is applied directly. This original diff is likely to be more minimal, but semantically less meaningful and could misrepresent edits, but the output will remain proper. Co-authored-by: danluu <danluu@git.wordpress.org> Co-authored-by: dmsnell <dmsnell@git.wordpress.org> Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org> Co-authored-by: alecgeatches <alecgeatches@git.wordpress.org>
1 parent dfc4c74 commit 54af1ce

13 files changed

Lines changed: 1192 additions & 157 deletions

File tree

‎packages/block-editor/src/store/actions.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ export const validateBlocksToTemplate =
9090
*
9191
* This type is duplicated to avoid creating circular dependencies.
9292
*
93-
* @see {import("@wordpress/core-data/src/types").WPBlockSelection}
9493
* @see {import("@wordpress/block-editor/src/store/selectors").WPBlockSelection}
94+
* @see {import("@wordpress/core-data/src/types").WPBlockSelection}
9595
* @see {import("@wordpress/editor/src/store/selectors").WPBlockSelection}
9696
*
9797
* @typedef {Object} WPBlockSelection

‎packages/core-data/src/awareness/post-editor-awareness.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import {
1616
LOCAL_CURSOR_UPDATE_DEBOUNCE_IN_MS,
1717
} from './config';
1818
import { STORE_NAME as coreStore } from '../name';
19-
import { htmlIndexToRichTextOffset } from '../utils/crdt-utils';
19+
import {
20+
asHtmlStringIndex,
21+
htmlIndexToRichTextOffset,
22+
} from '../utils/crdt-utils';
2023
import {
2124
areSelectionsStatesEqual,
2225
getSelectionState,
@@ -291,7 +294,7 @@ export class PostEditorAwareness extends BaseAwarenessState< PostEditorState > {
291294
return {
292295
richTextOffset: htmlIndexToRichTextOffset(
293296
absolutePosition.type.toString(),
294-
absolutePosition.index
297+
asHtmlStringIndex( absolutePosition.index )
295298
),
296299
localClientId,
297300
};

‎packages/core-data/src/test/rtc-rich-text-offset-space.test.js‎

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
/**
2+
* External dependencies
3+
*/
4+
import { act, render, waitFor } from '@testing-library/react';
5+
import {
6+
afterEach,
7+
beforeEach,
8+
describe,
9+
expect,
10+
it,
11+
jest,
12+
} from '@jest/globals';
13+
14+
/**
15+
* WordPress dependencies
16+
*/
17+
import {
18+
getBlockTypes,
19+
registerBlockType,
20+
unregisterBlockType,
21+
} from '@wordpress/blocks';
22+
import { RichText } from '@wordpress/block-editor';
23+
import { createRegistry, RegistryProvider } from '@wordpress/data';
24+
import { Y } from '@wordpress/sync';
25+
26+
/**
27+
* Mock sync manager accessor.
28+
*/
29+
jest.mock( '../sync', () => ( {
30+
...jest.requireActual( '../sync' ),
31+
getSyncManager: jest.fn(),
32+
LOCAL_EDITOR_ORIGIN: 'local-editor',
33+
} ) );
34+
35+
/**
36+
* Internal dependencies
37+
*/
38+
import { store as coreDataStore } from '../index';
39+
import { CRDT_RECORD_MAP_KEY, getSyncManager } from '../sync';
40+
import useEntityBlockEditor from '../hooks/use-entity-block-editor';
41+
import { applyPostChangesToCRDTDoc } from '../utils/crdt';
42+
import { getRootMap } from '../utils/crdt-utils';
43+
44+
const mockGetSyncManager = jest.mocked( getSyncManager );
45+
46+
const postTypeConfig = {
47+
kind: 'postType',
48+
name: 'post',
49+
baseURL: '/wp/v2/posts',
50+
transientEdits: { blocks: true, selection: true },
51+
mergedEdits: { meta: true },
52+
rawAttributes: [ 'title', 'excerpt', 'content' ],
53+
syncConfig: {},
54+
};
55+
56+
const postTypeEntity = {
57+
slug: 'post',
58+
rest_base: 'posts',
59+
labels: {
60+
item_updated: 'Updated Post',
61+
item_published: 'Post published',
62+
item_reverted_to_draft: 'Post reverted to draft.',
63+
},
64+
};
65+
66+
const OLD_HTML = '<em>italic</em><em>italic</em>';
67+
const NEW_HTML = '<em>italic</em>beta';
68+
const TEXT_OFFSET = 10;
69+
const SYNCED_PROPERTIES = new Set( [ 'blocks' ] );
70+
71+
function createRegistryWithStores() {
72+
const registry = createRegistry();
73+
74+
registry.register( coreDataStore );
75+
registry.dispatch( coreDataStore ).addEntities( [ postTypeConfig ] );
76+
registry
77+
.dispatch( coreDataStore )
78+
.receiveEntityRecords( 'root', 'postType', [ postTypeEntity ] );
79+
registry
80+
.dispatch( coreDataStore )
81+
.receiveEntityRecords( 'postType', 'post', [
82+
{
83+
id: 1,
84+
type: 'post',
85+
content: {
86+
raw: `<!-- wp:paragraph --><p>${ OLD_HTML }</p><!-- /wp:paragraph -->`,
87+
rendered: `<p>${ OLD_HTML }</p>`,
88+
},
89+
meta: {},
90+
},
91+
] );
92+
93+
return registry;
94+
}
95+
96+
function readFirstBlockContentFromDoc( doc ) {
97+
const ymap = getRootMap( doc, CRDT_RECORD_MAP_KEY );
98+
const yblocks = ymap.get( 'blocks' );
99+
const attrs = yblocks.get( 0 ).get( 'attributes' );
100+
return attrs.get( 'content' ).toString();
101+
}
102+
103+
describe( 'useEntityBlockEditor RTC rich-text offset-space bug', () => {
104+
let crdtDoc;
105+
106+
beforeEach( () => {
107+
crdtDoc = new Y.Doc();
108+
109+
registerBlockType( 'core/paragraph', {
110+
apiVersion: 3,
111+
title: 'Paragraph',
112+
category: 'text',
113+
attributes: {
114+
content: {
115+
type: 'rich-text',
116+
source: 'rich-text',
117+
selector: 'p',
118+
role: 'content',
119+
},
120+
},
121+
edit: () => null,
122+
save: ( { attributes } ) => (
123+
<p>
124+
<RichText.Content value={ attributes.content } />
125+
</p>
126+
),
127+
} );
128+
129+
mockGetSyncManager.mockReturnValue( {
130+
update: jest.fn( ( _objectType, _objectId, changes ) => {
131+
applyPostChangesToCRDTDoc(
132+
crdtDoc,
133+
changes,
134+
SYNCED_PROPERTIES
135+
);
136+
} ),
137+
} );
138+
} );
139+
140+
afterEach( () => {
141+
crdtDoc.destroy();
142+
mockGetSyncManager.mockReset();
143+
if (
144+
getBlockTypes().some( ( block ) => block.name === 'core/paragraph' )
145+
) {
146+
unregisterBlockType( 'core/paragraph' );
147+
}
148+
} );
149+
150+
it( 'preserves formatted paragraph content when onInput forwards blocks plus selection', async () => {
151+
const registry = createRegistryWithStores();
152+
let blocks;
153+
let onInput;
154+
155+
const TestComponent = () => {
156+
[ blocks, onInput ] = useEntityBlockEditor( 'postType', 'post', {
157+
id: 1,
158+
} );
159+
160+
return <div />;
161+
};
162+
163+
render(
164+
<RegistryProvider value={ registry }>
165+
<TestComponent />
166+
</RegistryProvider>
167+
);
168+
169+
await waitFor( () => expect( blocks ).toHaveLength( 1 ) );
170+
171+
const selection = {
172+
selectionStart: {
173+
clientId: blocks[ 0 ].clientId,
174+
attributeKey: 'content',
175+
offset: TEXT_OFFSET,
176+
},
177+
selectionEnd: {
178+
clientId: blocks[ 0 ].clientId,
179+
attributeKey: 'content',
180+
offset: TEXT_OFFSET,
181+
},
182+
};
183+
184+
act( () => {
185+
onInput( blocks, { selection } );
186+
} );
187+
188+
const nextBlocks = [
189+
{
190+
...blocks[ 0 ],
191+
attributes: {
192+
...blocks[ 0 ].attributes,
193+
content: NEW_HTML,
194+
},
195+
},
196+
];
197+
198+
act( () => {
199+
onInput( nextBlocks, { selection } );
200+
} );
201+
202+
expect( readFirstBlockContentFromDoc( crdtDoc ) ).toBe( NEW_HTML );
203+
} );
204+
} );

‎packages/core-data/src/utils/block-selection-history.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { Y } from '@wordpress/sync';
1010
* Internal dependencies
1111
*/
1212
import {
13+
asRichTextOffset,
1314
findBlockByClientIdInDoc,
1415
richTextOffsetToHtmlIndex,
1516
} from './crdt-utils';
@@ -166,7 +167,10 @@ function convertWPBlockSelectionToSelection(
166167
const offset = selection.offset ?? 0;
167168
const relativePosition = Y.createRelativePositionFromTypeIndex(
168169
changedYText,
169-
richTextOffsetToHtmlIndex( changedYText.toString(), offset )
170+
richTextOffsetToHtmlIndex(
171+
changedYText.toString(),
172+
asRichTextOffset( offset )
173+
)
170174
);
171175

172176
return {

0 commit comments

Comments
 (0)