Skip to content

fix: normalize data store attributes to plain strings in JS bridge#429

Merged
jkmassel merged 5 commits intotrunkfrom
jkmassel/fix-title-object-unwrap
Apr 8, 2026
Merged

fix: normalize data store attributes to plain strings in JS bridge#429
jkmassel merged 5 commits intotrunkfrom
jkmassel/fix-title-object-unwrap

Conversation

@jkmassel
Copy link
Copy Markdown
Contributor

@jkmassel jkmassel commented Apr 6, 2026

Summary

Fixes a bug where editor.getTitleAndContent() could return JavaScript objects instead of plain strings to the native host app, corrupting the content in the host app.

Root Cause

WordPress's Gutenberg data store has an architectural inconsistency in how getEditedPostAttribute handles values from the edits layer vs the base entity record.

When a post is loaded, title and content are stored as { raw: "..." } objects in the entity record (this is the shape getPost() in bridge.js constructs). Two different normalization paths exist:

  1. Base record path (no user edits): getEditedPostAttribute('title')getCurrentPostAttributegetCurrentPostgetRawEntityRecord (extracts .raw) → then getPostRawValue is applied — returns a plain string. ✅
  2. Edits path (after any code calls editEntityRecord): getEditedPostAttribute('title')edits[attributeName] — returns whatever was stored in edits, with no normalization. ❌

The edits reducer stores values verbatim:

case 'EDIT_ENTITY_RECORD':
    const nextEdits = { ...state[action.recordId], ...action.edits };

So if anything dispatches editEntityRecord('postType', type, id, { title: { raw: '...', rendered: '...' } }), the {raw, rendered} object goes directly into edits.title, and getEditedPostAttribute('title') returns the object — bypassing getPostRawValue entirely.

Similarly, getEditedPostContent() has a code path that returns record.content from getEditedEntityRecord without normalization — if the content in the edits layer is an object, it passes through as-is.

When does this happen in practice?

  • Undo/redo: The undo manager stores { from: editedRecord[key], to: edits[key] }. If at any point the edited record contained a {raw, rendered} object, undo replays it as an edit — re-introducing the object.
  • Any internal Gutenberg code that calls editEntityRecord with REST API-shaped data rather than plain strings.

Confirmed via E2E

We confirmed this by injecting {raw, rendered} objects via editEntityRecord in a Playwright test and then calling getTitleAndContent(), verifying the bridge returns plain strings (i.e., normalizeAttribute() catches the object before it reaches the host). The underlying getEditedPostAttribute('title') selector does return the object in this scenario — confirmed separately during reproduction testing.

We could not reproduce the bug through normal user interactions (typing, undo/redo, cache invalidation, REST API re-fetches) — the standard Gutenberg code paths happen to always produce string edits. The bug requires an internal code path that dispatches object-shaped edits, which is a Gutenberg data store implementation detail outside our control.

The Android/iOS host apps are not the source

Both the Android (GBKitGlobal.Post.title: String) and iOS host apps pass plain strings to GutenbergKit. The WordPress Android app (WordPress-Android) also decomposes {raw, rendered} from the REST API into plain strings via FluxC PostModel before reaching GutenbergKit. The issue is entirely within the JS data store layer.

What We Explored

1. Making native types match the {raw, rendered} shape

We initially updated iOS and Android to parse {raw, rendered} objects. This worked mechanically, but rendered is useless client-side — it's only available from the server's initial REST API response and becomes stale after any edit. The server-side rendering pipeline cannot be reproduced in the client.

2. Hoisting the {raw, rendered} wrapper into native input types

We considered having EditorConfiguration accept {raw, rendered} objects. But the data store replaces them with plain strings after edits, so output normalization would still be needed — making the input change pure overhead.

3. Using getEditedPostAttribute('content') instead of getEditedPostContent()

For 'content' specifically, getEditedPostAttribute delegates to getEditedPostContent() — same code path, same bug.

Fix

Add a normalizeAttribute() function at the JS bridge boundary that extracts .raw from objects or passes strings through unchanged. Applied to both title and content in getTitleAndContent(), to block content in appendTextAtCursor(), and defensively to the initial postTitleRef and postContentRef initialization.

function normalizeAttribute( value ) {
    if ( value === null || value === undefined ) {
        return '';
    }
    if ( typeof value === 'object' ) {
        return value.raw ?? '';
    }
    return String( value );
}

The getContent() JS bridge method and iOS EditorViewController.getContent() are retained as convenience accessors for contexts where only content is needed (e.g. comment editors with no title field). Both delegate to getTitleAndContent() so normalization happens in one place.

Changes

  • src/components/editor/use-host-bridge.js: Add normalizeAttribute(), apply to getTitleAndContent(), getContent(), and appendTextAtCursor(), defensively normalize ref initialization
  • src/components/editor/test/use-host-bridge.test.jsx: Unit tests for normalization edge cases (null, undefined, objects, arrays, primitives), changed flag correctness, getContent(), and appendTextAtCursor with object-shaped and string block content
  • e2e/get-title-and-content.spec.js: E2E tests covering plain string return in all states, including regression tests that inject {raw, rendered} objects via editEntityRecord and verify normalization with correct changed flag behavior
  • e2e/editor-page.js: Add getTitleAndContent() helper to page object
  • ios/.../EditorViewController.swift: getContent() now delegates to getTitleAndContent() for consistent normalization

Test Plan

  • JS unit tests pass (16 tests)
  • ESLint passes
  • E2E tests pass (6 tests) — including regression tests that reproduce the bug by injecting {raw, rendered} objects into the data store
  • iOS Swift package builds

@github-actions github-actions bot added the [Type] Bug An existing feature does not function as intended label Apr 6, 2026
@jkmassel jkmassel changed the title fix: unwrap title object in getTitleAndContent bridge method fix: return title as {raw, rendered} object from getTitleAndContent Apr 6, 2026
@jkmassel jkmassel force-pushed the jkmassel/fix-title-object-unwrap branch 8 times, most recently from abb6056 to 9c68308 Compare April 7, 2026 00:06
@jkmassel jkmassel changed the title fix: return title as {raw, rendered} object from getTitleAndContent fix: normalize data store attributes to plain strings in JS bridge Apr 7, 2026
@jkmassel jkmassel requested a review from dcalhoun April 7, 2026 02:18
The WordPress data store's `getEditedPostContent()` may return
`{raw, rendered}` objects instead of plain strings because it uses
`getEditedEntityRecord` (which preserves the object shape) rather than
`getRawEntityRecord` (which extracts `.raw`).

Add `normalizeAttribute()` to always extract the raw string before
returning values to the native host via `getTitleAndContent()`.

Also remove the redundant `getContent()` bridge method and its iOS
public API — `getTitleAndContent()` is the single accessor for editor
state.
@jkmassel jkmassel force-pushed the jkmassel/fix-title-object-unwrap branch from 9c68308 to aeb7f66 Compare April 7, 2026 02:19
@jkmassel jkmassel self-assigned this Apr 7, 2026
- Make normalizeAttribute explicitly handle null/undefined instead of
  relying on typeof null === 'object' with optional chaining
- Coerce non-string primitives to strings via String()
- Normalize postTitleRef/postContentRef initialization defensively
- Normalize block content in appendTextAtCursor
- Add unit tests for edge cases: null, undefined, {raw: null},
  {raw: undefined}, arrays, non-string primitives
- Add unit tests for changed flag correctness with object values
- Add unit tests for appendTextAtCursor (object content, string
  content, no block selected, unsupported block type)
- Add changed flag assertions to E2E object-injection regression tests
Copy link
Copy Markdown
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Left a few thoughts, but no blockers.

}

/// Returns the current editor content.
public func getContent() async throws -> String {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears there is one location using this method that we need to update: the unused comment editor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting point – I've put it back with a note explaining why it's there to avoid a breaking change in 3181e08.

Comment on lines +47 to +77
vi.mock( '@wordpress/core-data', () => ( {
store: { name: 'core' },
} ) );
vi.mock( '@wordpress/editor', () => ( {
store: { name: 'core/editor' },
} ) );
vi.mock( '@wordpress/blocks', () => ( {
parse: vi.fn( () => [] ),
serialize: vi.fn( () => '' ),
getBlockType: vi.fn(),
} ) );
vi.mock( '@wordpress/rich-text', () => ( {
create: vi.fn( ( { html } ) => ( {
text: html,
formats: [],
replacements: [],
start: 0,
end: html.length,
} ) ),
insert: vi.fn( ( value, text ) => ( {
text: value.text + text,
formats: [],
replacements: [],
start: 0,
end: value.text.length + text.length,
} ) ),
toHTMLString: vi.fn( ( { value } ) => value.text ),
} ) );
vi.mock( '@wordpress/block-editor', () => ( {
store: { name: 'core/block-editor' },
} ) );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we simplify the mocks in this file by reusing the base mocks in __mock__. Some need to be expanded with sensible defaults.

You could copy the below diff and pbpaste | git apply.

diff --git a/__mocks__/@wordpress/block-editor.js b/__mocks__/@wordpress/block-editor.js
index 7f90f182..12d53b27 100644
--- a/__mocks__/@wordpress/block-editor.js
+++ b/__mocks__/@wordpress/block-editor.js
@@ -1 +1,3 @@
 // Intentionally empty — prevents the real module from loading.
+
+export const store = { name: 'core/block-editor' };
diff --git a/__mocks__/@wordpress/core-data.js b/__mocks__/@wordpress/core-data.js
index 7f90f182..046ff29f 100644
--- a/__mocks__/@wordpress/core-data.js
+++ b/__mocks__/@wordpress/core-data.js
@@ -1 +1 @@
-// Intentionally empty — prevents the real module from loading.
+export const store = { name: 'core/data' };
diff --git a/src/components/editor/test/use-host-bridge.test.jsx b/src/components/editor/test/use-host-bridge.test.jsx
index 14ea7819..1a3fea16 100644
--- a/src/components/editor/test/use-host-bridge.test.jsx
+++ b/src/components/editor/test/use-host-bridge.test.jsx
@@ -44,17 +44,9 @@ vi.mock( '@wordpress/data', () => ( {
 		selectionChange: mockSelectionChange,
 	} ),
 } ) );
-vi.mock( '@wordpress/core-data', () => ( {
-	store: { name: 'core' },
-} ) );
-vi.mock( '@wordpress/editor', () => ( {
-	store: { name: 'core/editor' },
-} ) );
-vi.mock( '@wordpress/blocks', () => ( {
-	parse: vi.fn( () => [] ),
-	serialize: vi.fn( () => '' ),
-	getBlockType: vi.fn(),
-} ) );
+vi.mock( '@wordpress/core-data' );
+vi.mock( '@wordpress/editor' );
+vi.mock( '@wordpress/blocks' );
 vi.mock( '@wordpress/rich-text', () => ( {
 	create: vi.fn( ( { html } ) => ( {
 		text: html,
@@ -72,9 +64,7 @@ vi.mock( '@wordpress/rich-text', () => ( {
 	} ) ),
 	toHTMLString: vi.fn( ( { value } ) => value.text ),
 } ) );
-vi.mock( '@wordpress/block-editor', () => ( {
-	store: { name: 'core/block-editor' },
-} ) );
+vi.mock( '@wordpress/block-editor' );
 
 const defaultPost = {
 	id: 1,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that is nicer, thanks! Done in 55ef73a

const mockGetSelectionEnd = vi.fn();
const mockUpdateBlock = vi.fn();
const mockSelectionChange = vi.fn();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mocking the project's logger will mitigate test output noise.

Suggested change
vi.mock( '../../../utils/logger' );
Image

Comment on lines +226 to +228
if ( typeof value === 'object' ) {
return value.raw ?? '';
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem unlikely, but I'll point out....

typeof [] === 'object'; we could add an explicit check with Array.isArray() if desired.

{ raw: { raw: 'some string' } } would return an object; we could recursively call normalizeAttribute( value.raw ) if desired. But I suppose the could create an infinite loop in really odd circumstances.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the array case is handled but it's pretty non-idiomatic. Addressed in acf1a5b.

{ raw: { raw: 'some string' } }

Besides an already-corrupted post, any other case where this could happen? I'm inclined to avoid handling this case but curious if there's something I overlooked?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine to forgo addressing the nested object. It seems improbable.

Restore the getContent() bridge method on both JS and iOS. It is used
in contexts where the editor has no title field (e.g. comment editors).

Both implementations delegate to getTitleAndContent() so normalization
happens in one place.
@jkmassel jkmassel marked this pull request as ready for review April 8, 2026 17:27
@jkmassel jkmassel enabled auto-merge (squash) April 8, 2026 17:43
@jkmassel jkmassel merged commit b5b75c2 into trunk Apr 8, 2026
16 checks passed
@jkmassel jkmassel deleted the jkmassel/fix-title-object-unwrap branch April 8, 2026 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants