fix: render fidelity for code-block-pro font and core/list values - #70
Merged
Conversation
…mily The code-block-pro enricher emitted `font-family:<name>` with no generic family, so a custom CBP font (e.g. Code-Pro-JetBrains-Mono, which is not a loaded webfont) fell back to the browser default serif. Append the monospace stack when the value has no generic family keyword, mirroring what the CBP editor bakes; idempotent, so re-runs never double-append. Claude-Session: https://claude.ai/code/session_017AMC5bVpEM9G3WtXbXtmwj
Inserting a core/list with its <li> items in the deprecated `values` attribute and an empty <ul> wrapper (no core/list-item children) saved a block that renders an empty list on modern WordPress. Add a write-path normalizer that bakes the items into the wrapper (reconciling ordered vs unordered), modeled on the core/image normalizer and running at the non-bypassable gk/block-mcp/block/normalize chokepoint. Idempotent, with guards that leave correct and genuinely-deprecated lists byte-identical. Claude-Session: https://claude.ai/code/session_017AMC5bVpEM9G3WtXbXtmwj
WalkthroughThe change adds monospace fallback and wrapper synchronization for Code Block Pro blocks. It also adds normalization for malformed ChangesCode Block Pro font synchronization
Core list normalization
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The embedded dist bundle (assets/mcp-server/index.cjs) is committed and verified against a fresh build in CI. Rebuild it so it includes the ensureMonospaceFallback change from the enrichers fix. Claude-Session: https://claude.ai/code/session_017AMC5bVpEM9G3WtXbXtmwj
The code-block-pro enricher has two innerHTML paths. The fresh-wrapper path runs ensureMonospaceFallback() on the font stack; the in-place path rewrites only the <pre> and the copy <textarea>, leaving the wrapper's style attribute as serialized. Two failures follow for any block that already has markup: - A fontFamily attribute change saves but never reaches the rendered wrapper. - A stack with no generic family (a bare `Code-Pro-JetBrains-Mono`) falls back to the browser default serif when that webfont is unavailable, so code renders in a proportional face. The early bail-out compounded it: a fontFamily-only edit arrives with identical codeHTML and language, returned null, and skipped the wrapper entirely. syncWrapperFontFamily() rewrites just the font-family declaration and the data-code-block-pro-font-family attribute. It deliberately does not rebuild the style attribute, which also carries CBP's --cbp-* and --shiki-* custom properties. The bail-out now attempts the sync before returning null. Five regression tests; four fail with the fix reverted.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php (2)
97-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant
innerBlockscheck.Line 73 returns early for any block with non-empty
innerBlocks. At line 98 the condition is therefore always true. AssigninnerContentunconditionally.♻️ Proposed refactor
$block['innerHTML'] = $html; - if ( empty( $block['innerBlocks'] ) ) { - $block['innerContent'] = array( $html ); - } + // The block is a leaf here (non-empty innerBlocks returned early), so a + // single innerContent chunk keeps the null-placeholder invariant. + $block['innerContent'] = array( $html );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php` around lines 97 - 100, In the normalizer method containing the `$block['innerHTML']` assignment, remove the redundant `empty( $block['innerBlocks'] )` condition and assign `$block['innerContent']` to `array( $html )` unconditionally, relying on the earlier return for blocks with non-empty innerBlocks.
38-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign two details with the project conventions.
- Line 41 states that a normalizer loader calls
init(), but line 220 registers the filter when the file loads. Describe the actual registration path.- Line 90 puts a function-call result directly in
if, and line 198 puts a compound check directly inif. Assign both to named variables. Thewhile ( $processor->next_tag() )loops are load-bearing and stay as they are.♻️ Proposed refactor
- if ( self::contains_li( $html ) ) { + $has_li = self::contains_li( $html ); + if ( $has_li ) { return $block; }- if ( isset( $attrs['start'] ) && is_scalar( $attrs['start'] ) ) { + $has_start = isset( $attrs['start'] ) && is_scalar( $attrs['start'] ); + if ( $has_start ) { $processor->set_attribute( 'start', (string) $attrs['start'] ); }As per coding guidelines: "Assign function-call results and compound checks to named variables before using them in
if,while, or ternary expressions, except for load-bearing short-circuit guards." and "Comments must describe present-tense behavior and hard contracts only".Also applies to: 90-90, 198-198
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php` around lines 38 - 49, Update the init() docblock to describe the actual file-load filter registration path instead of claiming a normalizer loader invokes it. In the relevant normalization logic, assign the function-call result at line 90 and the compound condition at line 198 to descriptive variables before their if statements; leave the load-bearing processor->next_tag() while loops unchanged.Source: Coding guidelines
wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php (1)
536-552: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the ordered-list coverage.
test_ordered_list_with_ul_supplied_emits_olassertsstartonly.prepare_wrapper()also appliestypeandreversed. Add assertions for both so a regression in either branch fails the suite.💚 Proposed addition
array( 'values' => self::INVALID_LIST_VALUES, 'ordered' => true, 'start' => 3, + 'type' => 'A', + 'reversed' => true, ), self::INVALID_LIST_HTML ); @@ $this->assertStringContainsString( 'start="3"', $out[0]['innerHTML'], 'the ordered start attribute must be carried onto the <ol>' ); + $this->assertStringContainsString( 'type="A"', $out[0]['innerHTML'], 'the ordered type attribute must be carried onto the <ol>' ); + $this->assertStringContainsString( 'reversed', $out[0]['innerHTML'], 'the reversed attribute must be carried onto the <ol>' );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php` around lines 536 - 552, Extend test_ordered_list_with_ul_supplied_emits_ol to configure ordered-list type and reversed attributes, then assert the normalized innerHTML contains both corresponding attributes alongside the existing start assertion. Keep the current wrapper and list-item assertions unchanged.src/__tests__/unit/enrichers/cbp-enricher.test.ts (1)
569-575: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the synchronized data attribute.
blockWithWrapper()has nodata-code-block-pro-font-familyattribute. This test only verifies thestyledeclaration. If a change stops adding the data attribute during a font-only update, the test still passes.Add an assertion for
data-code-block-pro-font-family="Menlo,monospace"here or in the font-only update test.Proposed test assertion
expect(result.innerHTML).toContain('font-family:Menlo,monospace;font-size:1rem'); + expect(result.innerHTML).toContain( + 'data-code-block-pro-font-family="Menlo,monospace"', + ); expect(result.innerHTML).not.toContain('font-family:Code-Pro-JetBrains-Mono;');Based on PR objectives, the wrapper data attribute must remain synchronized when only
fontFamilychanges.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/unit/enrichers/cbp-enricher.test.ts` around lines 569 - 575, Add an assertion to the stale-wrapper font-family test around enrichBlock and blockWithWrapper, verifying the result includes data-code-block-pro-font-family="Menlo,monospace" alongside the existing style assertions. Ensure the test covers synchronization when only fontFamily changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/enrichers.ts`:
- Around line 265-268: Update ensureMonospaceFallback to detect generic font
families only when they are complete comma-separated entries, not substrings
within custom family names such as custom-monospace-font. Preserve the existing
fallback behavior for lists without a recognized generic family and return the
original fontFamily unchanged when a valid generic entry is present.
In `@wordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjs`:
- Around line 52352-52355: Update ensureMonospaceFallback to parse the
comma-separated fontFamily list, normalize each family entry, and compare
complete entries against the generic family names rather than using substring
matching. Preserve existing generic-family detection and fallback behavior,
while ensuring custom names such as those containing “serif” or “monospace”
still receive the fallback. Add regression coverage for both cases.
- Line 52414: Update the generated-markup style construction around
ensureMonospaceFallback so fontFamily is added only when it is a non-empty,
non-whitespace string, matching the synchronization path’s existing absence
check; otherwise skip the font-family declaration.
- Around line 52363-52375: Validate the string-valued fontFamily attribute as a
permitted CSS font-family value before passing it to ensureMonospaceFallback or
interpolating it into markup. Reject values containing CSS delimiters or
otherwise invalid syntax, preserve the existing fallback behavior for valid
values, and regenerate the bundled index.cjs after updating the source
implementation.
In
`@wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php`:
- Line 29: Replace every `@since TODO` annotation in the public class and its
`init()` and `normalize()` methods with `@since 2.1.0`, covering all three
documented locations.
- Around line 149-153: Update the fallback branch in the core list normalizer
around $close and $pos to append the missing closing tag to the prepared
$wrapper, returning $wrapper . $close instead of rebuilding the opening tag.
Preserve all attributes and classes applied by prepare_wrapper().
- Around line 94-99: Sanitize the complete HTML returned by
bake_values_into_wrapper() before assigning it to $block['innerHTML'] in the
core list normalizer. Apply the same final-output sanitization on both return
paths, including the fallback when innerBlocks is empty, while leaving
attributes['values'] unchanged.
---
Nitpick comments:
In `@src/__tests__/unit/enrichers/cbp-enricher.test.ts`:
- Around line 569-575: Add an assertion to the stale-wrapper font-family test
around enrichBlock and blockWithWrapper, verifying the result includes
data-code-block-pro-font-family="Menlo,monospace" alongside the existing style
assertions. Ensure the test covers synchronization when only fontFamily changes.
In
`@wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php`:
- Around line 97-100: In the normalizer method containing the
`$block['innerHTML']` assignment, remove the redundant `empty(
$block['innerBlocks'] )` condition and assign `$block['innerContent']` to
`array( $html )` unconditionally, relying on the earlier return for blocks with
non-empty innerBlocks.
- Around line 38-49: Update the init() docblock to describe the actual file-load
filter registration path instead of claiming a normalizer loader invokes it. In
the relevant normalization logic, assign the function-call result at line 90 and
the compound condition at line 198 to descriptive variables before their if
statements; leave the load-bearing processor->next_tag() while loops unchanged.
In `@wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php`:
- Around line 536-552: Extend test_ordered_list_with_ul_supplied_emits_ol to
configure ordered-list type and reversed attributes, then assert the normalized
innerHTML contains both corresponding attributes alongside the existing start
assertion. Keep the current wrapper and list-item assertions unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82ef67be-ab90-4366-ac5b-ef1de1107925
📒 Files selected for processing (6)
src/__tests__/unit/enrichers/cbp-enricher.test.tssrc/enrichers.tswordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjswordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.phpwordpress-plugin/gk-block-mcp/readme.txtwordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php
| /** | ||
| * Normalizer for core/list blocks. | ||
| * | ||
| * @since TODO |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace @since TODO with @since 2.1.0.
The class, init(), and normalize() are shipped public members. The project convention requires a concrete version placeholder until the release version is settled.
📝 Proposed fix
- * `@since` TODO
+ * `@since` 2.1.0Apply the same change at lines 43 and 60.
As per coding guidelines: "Add @since {version} to shipped public production classes, methods, hooks, and REST routes; new code uses @since 2.1.0 until the release version is settled."
Also applies to: 43-43, 60-60
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php`
at line 29, Replace every `@since TODO` annotation in the public class and its
`init()` and `normalize()` methods with `@since 2.1.0`, covering all three
documented locations.
Source: Coding guidelines
Generic font families are matched as whole comma-separated entries. A
substring test read `Source Serif 4` and `custom-monospace-font` as
already carrying a generic family and withheld the fallback stack those
names most need.
A font-family that is blank or carries CSS structure is now dropped
rather than emitted. escapeAttr stops a value breaking out of the
attribute, but the declaration is spliced among others, so an unescaped
`;` or `{` appended CSS of the caller's choosing.
The list normalizer sanitizes the markup it bakes. `values` is attribute
data and normalization runs after the write path's innerHTML
sanitization, so a `<script>` element or an `onclick` handler supplied
through that attribute reached post_content intact, bypassing
wp_kses_post. The sanitized value is written back to the attribute too,
since Gutenberg matches the values-based deprecation by regenerating
markup from it and the two must agree. Both new tests fail against the
pre-fix normalizer with exactly those symptoms.
An unclosed wrapper now keeps the class and ordered attributes
prepare_wrapper applied, instead of being rebuilt as a bare opening tag.
Also: the redundant innerBlocks re-check, two inlined conditionals, and
ordered-list coverage for type and reversed.
@SInCE is 2.2.1, not the 2.1.0 the review suggested: 2.1.0 shipped
without this class, which is in no tag yet.
Member
Author
|
Worked through all 7 findings in 2b86cf4. Six applied, one deviated from, one skipped. Applied
Deviated: Used Skipped: "Called once at plugin init by the normalizer loader" is verbatim what the shipped sibling normalizer says, and the loader is what causes the call — On the data attribute Adding Note on an existing test
Verification New tests were confirmed to fail against the pre-fix normalizer with the exact symptoms (script and handler in |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php (1)
393-403: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace change-history comments with current contracts.
Keep comments limited to the malformed input and required normalized output.
wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php#L393-L403: describe the input shape and the required baked wrapper result. Remove reported-issue and author-history text.wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php#L16-L17: state thevaluesand baked-markup synchronization invariant. Remove next-edit rationale.wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php#L558-L570: state that stored and attribute values must be sanitized. Remove bypass chronology.wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php#L599-L605: state the required result for an unclosed wrapper. Remove prior fallback behavior.As per coding guidelines: “Comments must describe present-tense behavior and hard contracts only; omit history, journal entries, off-tree specification pointers, and future-architecture speculation.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php` around lines 393 - 403, Revise only the comments at wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php:393-403 to describe the malformed core/list input and required baked-wrapper output; at wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php:16-17 to state the values/markup synchronization invariant; at wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php:558-570 to state that stored and attribute values are sanitized; and at wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php:599-605 to state the required result for an unclosed wrapper. Remove issue history, author rationale, next-edit rationale, bypass chronology, and prior fallback behavior while preserving present-tense contracts tied to the relevant core-list normalizer and tests.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php`:
- Around line 165-168: Update the closing-tag detection in the current
normalizer method and prepare_wrapper() to match valid list closing tags with
optional whitespace before “>”, preserving the existing wrapper attributes and
ensuring replacement tags remain balanced. Add coverage for a wrapper ending in
</ul > and verify list items are inserted before the closing tag.
- Around line 103-108: Update the normalization flow around
Block_Writer::sanitize_inner_html and bake_values_into_wrapper so it returns the
original block unchanged when sanitized $values contains no <li> element, even
if the string is non-empty. Preserve the existing list-processing path when at
least one list item remains, and add a regression test covering a non-list
fragment.
---
Nitpick comments:
In `@wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php`:
- Around line 393-403: Revise only the comments at
wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php:393-403 to
describe the malformed core/list input and required baked-wrapper output; at
wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php:16-17
to state the values/markup synchronization invariant; at
wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php:558-570 to
state that stored and attribute values are sanitized; and at
wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php:599-605 to
state the required result for an unclosed wrapper. Remove issue history, author
rationale, next-edit rationale, bypass chronology, and prior fallback behavior
while preserving present-tense contracts tied to the relevant core-list
normalizer and tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1f2cdca-ff5f-4c07-b8a9-b2f997aea9b1
📒 Files selected for processing (6)
src/__tests__/unit/enrichers/cbp-enricher.test.tssrc/enrichers.tswordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjswordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.phpwordpress-plugin/gk-block-mcp/readme.txtwordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php
🚧 Files skipped from review as they are similar to previous changes (3)
- wordpress-plugin/gk-block-mcp/assets/mcp-server/index.cjs
- wordpress-plugin/gk-block-mcp/readme.txt
- src/enrichers.ts
Three defects in the values-baking path, each with a regression test that fails when the fix is reverted: - `values` holding no <li> was spliced into the wrapper regardless, emitting a list whose children are not list items. The guard runs on the SANITIZED value, since a fragment can lose its only <li> to wp_kses_post(). - An end tag may carry whitespace before its `>`, so the literal `</ul>` search missed `</ul >`. bake_values_into_wrapper() then appended the items after the existing closing tag, outside the list, and prepare_wrapper() renamed an opening tag without its matching close, leaving `<ol …></ul >`. find_last_closing_tag() matches whitespace-tolerantly for both. - Only the `values` fragment was sanitized. The composed wrapper is now sanitized as a whole, so markup carried by the wrapper itself cannot reach post_content. Sanitization is idempotent, so the baked fragment still matches the attribute Gutenberg regenerates from. composer lint 0/0, composer analyze [OK], composer test green (1462/28/11/5).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php (1)
625-631: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove historical bug narratives from these docblocks.
Describe the current input-output contract only. Remove statements about prior checks, fallback behavior, and earlier malformed output.
wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php#L625-L631: State that non-listvaluescontent does not change the wrapper.wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php#L644-L647: State that sanitized content without<li>elements does not change the wrapper.wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php#L661-L666: State that a whitespace-bearing closing tag keeps baked items inside the wrapper.wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php#L686-L691: State that ordered-list reconciliation updates both wrapper tags.wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php#L709-L713: State that normalization sanitizes the composed wrapper markup.As per coding guidelines, comments must “describe present-tense behavior and hard contracts only” and omit historical narratives.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php` around lines 625 - 631, Update the docblocks for the affected BlockNormalizer tests to describe only current input-output contracts: at wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php lines 625-631, state that non-list values content leaves the wrapper unchanged; lines 644-647, state that sanitized content without li elements leaves it unchanged; lines 661-666, state that a whitespace-bearing closing tag keeps baked items inside the wrapper; lines 686-691, state that ordered-list reconciliation updates both wrapper tags; and lines 709-713, state that normalization sanitizes the composed wrapper markup. Remove all historical explanations about prior checks, fallback behavior, or malformed output.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php`:
- Around line 625-631: Update the docblocks for the affected BlockNormalizer
tests to describe only current input-output contracts: at
wordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php lines 625-631,
state that non-list values content leaves the wrapper unchanged; lines 644-647,
state that sanitized content without li elements leaves it unchanged; lines
661-666, state that a whitespace-bearing closing tag keeps baked items inside
the wrapper; lines 686-691, state that ordered-list reconciliation updates both
wrapper tags; and lines 709-713, state that normalization sanitizes the composed
wrapper markup. Remove all historical explanations about prior checks, fallback
behavior, or malformed output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0166407a-9606-41a6-839a-165fdf558d5b
📒 Files selected for processing (2)
wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.phpwordpress-plugin/gk-block-mcp/tests/Block/BlockNormalizerTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- wordpress-plugin/gk-block-mcp/includes/block-normalizers/class-core-list-normalizer.php
…idelity # Conflicts: # wordpress-plugin/gk-block-mcp/readme.txt
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
1 participant
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.
Fixes two cases where a block was saved with content the block's own render ignores, so it looked correct in the tool response but rendered wrong on the front end.
Font — code-block-pro rendered serif
src/enrichers.tsbakedfont-family:<name>with no generic family. A custom CBP font name (e.g.Code-Pro-JetBrains-Mono) is not a loaded webfont, so with no monospace fallback browsers fell back to the default serif. AddedensureMonospaceFallback(): when the value has no generic family keyword it appends,ui-monospace,SFMono-Regular,Menlo,Monaco,Consolas,monospace(what the CBP editor bakes), applied beforeescapeAttr. Idempotent — a word-boundarymonospacematch also catchesui-monospaceand any value already ending inmonospace, so re-runs never double-append. Only the build-from-scratch branch is touched.List — core/list saved an empty
<ul>Inserting a
core/listwith its<li>items in the deprecatedvaluesattribute and an empty<ul>wrapper (nocore/list-itemchildren) saved a block that renders an empty list on modern WordPress. Addedincludes/block-normalizers/class-core-list-normalizer.php, modeled on the core/image normalizer and registered on the non-bypassablegk/block-mcp/block/normalizewrite chokepoint. It bakes thevaluesitems into the wrapper (reconciling<ul>/<ol>from theorderedattribute, preserving nested sublists and inline links) and keeps the block a leaf (setsinnerContent). Three guards make it idempotent and never false-repair: it skips wheninnerBlocksis non-empty, whenvaluesis empty, or when the wrapper already contains an<li>. The detect signature (populatedvaluesin the comment attrs + empty wrapper + no<li>) is one no valid or deprecated Gutenberg serialization can produce.Why different layers
The MCP generates the CBP wrapper in TS (fix at the source there), but passes list markup through verbatim to PHP, where the write-path normalizer is the authoritative, non-bypassable place to repair it.
Tests
TDD (RED proven for both). Full gate green: TS
npm test891/891; PHPcomposer test1455 + 28 + 11 + 5 all OK;composer lint0/0;composer analyze(PHPStan L5) [OK]. Adversarially reviewed.Note
@since TODOon the new PHP class — 2.2.0 is already released and the next version isn't set; resolve to the shipped version at release.https://claude.ai/code/session_017AMC5bVpEM9G3WtXbXtmwj
💾 Build file (5808211).
Summary by CodeRabbit
Bug Fixes
Documentation