From aec9701656b03dc8092af53a9c705e3ee9ce8f74 Mon Sep 17 00:00:00 2001 From: Brandonma21 Date: Thu, 17 Sep 2026 10:57:09 +0200 Subject: [PATCH 1/7] Fix editor validation errors on Services pill/tag badges (LS-4207) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug fix - services-linked-decisions.php, services-service-clusters.php: add missing `has-border-color` class to the process-pill and cluster-tag wrapper groups - Root cause: both blocks set an inline border color via a custom JSON style attribute, but WordPress's border block support always expects a `has-border-color` class on the wrapper whenever a border color is set — the hand-authored static HTML never included it, so the editor's re-derived expected markup never matched the stored HTML, permanently flagging these blocks as "unexpected or invalid content" - Confirmed via a working control case in services-cta.php, which sets a border color the same way and already includes the class correctly --- patterns/sections/services-linked-decisions.php | 2 +- patterns/sections/services-service-clusters.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/patterns/sections/services-linked-decisions.php b/patterns/sections/services-linked-decisions.php index c27ba77..70b1d9c 100644 --- a/patterns/sections/services-linked-decisions.php +++ b/patterns/sections/services-linked-decisions.php @@ -99,7 +99,7 @@ ?> -
+

diff --git a/patterns/sections/services-service-clusters.php b/patterns/sections/services-service-clusters.php index 895fda0..50c8401 100644 --- a/patterns/sections/services-service-clusters.php +++ b/patterns/sections/services-service-clusters.php @@ -133,7 +133,7 @@ -
+
From e123ea2c48c17715d6be7233d83a8c0b7dc814b4 Mon Sep 17 00:00:00 2001 From: Brandonma21 Date: Thu, 17 Sep 2026 11:22:08 +0200 Subject: [PATCH 2/7] Add CHANGELOG entry for Services badge border-color fix (LS-4207) Docs - CHANGELOG.md: add dated entry for the has-border-color fix on services-linked-decisions.php and services-service-clusters.php, matching the one-entry-per-PR convention already used for the other Services page and mobile-menu entries --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e8908d..3c5f14d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 --- +## [Unreleased] — Fix editor validation errors on Services pill/tag badges (LS-4207) + +### Fixed + +- `patterns/sections/services-linked-decisions.php`, `patterns/sections/services-service-clusters.php`: added the missing `has-border-color` class to the `ls-process-pill` and `ls-cluster-tag` wrapper groups. Both blocks set an inline border colour via a custom JSON style attribute, but WordPress's border block support always expects a `has-border-color` class on the wrapper whenever a border colour is set. The hand-authored static HTML never included it, so the editor's re-derived expected markup never matched the stored HTML, permanently flagging these blocks as "Block contains unexpected or invalid content" with an "Attempt Recovery" prompt in the Site Editor. + +--- + ## [Unreleased] — Fix mobile menu: restore dropdown links, remove Systems, refine spacing (LS-3222) ### Fixed From b5e7a1597db8d1fefb828ecb0057e0ad69f149c2 Mon Sep 17 00:00:00 2001 From: Brandonma21 Date: Thu, 17 Sep 2026 11:26:21 +0200 Subject: [PATCH 3/7] Link PR #59 in CHANGELOG entry for LS-4207 Docs - CHANGELOG.md: add PR reference link now that the pull request exists, matching the existing per-PR link convention Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c5f14d..7f75150 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `patterns/sections/services-linked-decisions.php`, `patterns/sections/services-service-clusters.php`: added the missing `has-border-color` class to the `ls-process-pill` and `ls-cluster-tag` wrapper groups. Both blocks set an inline border colour via a custom JSON style attribute, but WordPress's border block support always expects a `has-border-color` class on the wrapper whenever a border colour is set. The hand-authored static HTML never included it, so the editor's re-derived expected markup never matched the stored HTML, permanently flagging these blocks as "Block contains unexpected or invalid content" with an "Attempt Recovery" prompt in the Site Editor. +([#59](https://github.com/lightspeedwp/ls-theme/pull/59)) + --- ## [Unreleased] — Fix mobile menu: restore dropdown links, remove Systems, refine spacing (LS-3222) From 8eb0be52d0d98ae6117cfb1ce16f4a95659ce01c Mon Sep 17 00:00:00 2001 From: Brandonma21 Date: Thu, 17 Sep 2026 11:36:08 +0200 Subject: [PATCH 4/7] Fix invalid Icon block width attribute and missing icon slug (LS-4207) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug fix - section-card-services.php, services-service-tiles.php: move the Icon block's width from an unsupported top-level "width" JSON attribute to the correct "style":{"dimensions":{"width":...}} path, matching core/icon's actual schema — the old attribute was silently dropped, leaving the icon unsized - section-card-services.php: fix icon slug "lightspeed/rocket-launch" to "lightspeed/rocket" — rocket-launch.svg does not exist in the ls-plugin icon library, so the icon failed to render entirely Co-Authored-By: Claude Sonnet 5 --- patterns/cards/section-card-services.php | 2 +- patterns/sections/services-service-tiles.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/patterns/cards/section-card-services.php b/patterns/cards/section-card-services.php index 9ec2244..950a8b8 100644 --- a/patterns/cards/section-card-services.php +++ b/patterns/cards/section-card-services.php @@ -17,7 +17,7 @@
- +
diff --git a/patterns/sections/services-service-tiles.php b/patterns/sections/services-service-tiles.php index 5b54b2f..ea84f7e 100644 --- a/patterns/sections/services-service-tiles.php +++ b/patterns/sections/services-service-tiles.php @@ -137,7 +137,7 @@
- +
From cb7ab287d2b9db36b7eb0cc61ea6ec7f6dafdee6 Mon Sep 17 00:00:00 2001 From: Brandonma21 Date: Thu, 17 Sep 2026 11:36:59 +0200 Subject: [PATCH 5/7] Update CHANGELOG entry for LS-4207 with icon fixes and PR #60 link Docs - CHANGELOG.md: expand the entry title/body to cover the Icon block width and slug fixes, and update the PR reference from #59 to #60 after the accidental branch-rename close Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f75150..0052660 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,13 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 --- -## [Unreleased] — Fix editor validation errors on Services pill/tag badges (LS-4207) +## [Unreleased] — Fix editor validation errors and Icon block attributes on Services patterns (LS-4207) ### Fixed - `patterns/sections/services-linked-decisions.php`, `patterns/sections/services-service-clusters.php`: added the missing `has-border-color` class to the `ls-process-pill` and `ls-cluster-tag` wrapper groups. Both blocks set an inline border colour via a custom JSON style attribute, but WordPress's border block support always expects a `has-border-color` class on the wrapper whenever a border colour is set. The hand-authored static HTML never included it, so the editor's re-derived expected markup never matched the stored HTML, permanently flagging these blocks as "Block contains unexpected or invalid content" with an "Attempt Recovery" prompt in the Site Editor. +- `patterns/cards/section-card-services.php`, `patterns/sections/services-service-tiles.php`: moved the Icon block's width from an unsupported top-level `"width"` JSON attribute to the correct `"style":{"dimensions":{"width":...}}` path, matching `core/icon`'s actual schema. The old attribute was silently dropped, leaving the icon unsized. +- `patterns/cards/section-card-services.php`: fixed icon slug `lightspeed/rocket-launch` to `lightspeed/rocket` — `rocket-launch.svg` does not exist in the `ls-plugin` icon library, so the icon failed to render entirely. -([#59](https://github.com/lightspeedwp/ls-theme/pull/59)) +([#60](https://github.com/lightspeedwp/ls-theme/pull/60)) --- From 1592bc356677759b41fb88a17afd1a8814f98e4b Mon Sep 17 00:00:00 2001 From: Brandonma21 Date: Thu, 17 Sep 2026 12:11:27 +0200 Subject: [PATCH 6/7] Fix false-positive console error on 404 template test (LS-2940) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug fix - Chromium logs its own console.error for the main document's non-2xx status (e.g. loading the 404 template), independent of any theme JS - browser-errors.ts had no exemption for this, so the standing suite's 404-page test always failed even though the page behaves correctly - network-errors.ts already exempts the equivalent main-frame response for the same reason; mirrored that exemption on the console side Context - Confirmed via retest that this was the only remaining console error under LS-2940 — the other 4 flagged pages (#244, #246, #247, #248) were already resolved by the earlier ls-plugin CSS fix (LS-2935) --- tests/helpers/browser-errors.ts | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/helpers/browser-errors.ts b/tests/helpers/browser-errors.ts index f9610bf..7d43a7f 100644 --- a/tests/helpers/browser-errors.ts +++ b/tests/helpers/browser-errors.ts @@ -16,6 +16,21 @@ function isAllowed(message: string): boolean { return ALLOWED_ERROR_PATTERNS.some((pattern) => pattern.test(message)); } +// Chromium logs this itself (independent of any theme JS) whenever the +// top-level document's own response is >= 400 — e.g. loading the 404 +// template. network-errors.ts already exempts the main-frame navigation +// response for the same reason (a route can legitimately return a non-2xx +// status); this mirrors that exemption for the console-side equivalent. +// For this specific browser-generated message, ConsoleMessage.location().url +// is the URL of the resource that failed — comparing it to the page's own +// current URL confirms it's the main document's own status, not a broken +// subresource (which would still be correctly caught). +const RESOURCE_STATUS_ERROR_PATTERN = /^Failed to load resource: the server responded with a status of \d+/; + +function isExpectedMainDocumentStatusError(message: import('@playwright/test').ConsoleMessage, page: Page): boolean { + return RESOURCE_STATUS_ERROR_PATTERN.test(message.text()) && message.location().url === page.url(); +} + /** * Installs console/pageerror listeners on `page` and returns a collector. * Must be called before navigation to catch errors from the initial load. @@ -24,9 +39,10 @@ export function watchBrowserErrors(page: Page): BrowserErrorCollector { const collector: BrowserErrorCollector = { consoleErrors: [], pageErrors: [] }; page.on('console', (message) => { - if (message.type() === 'error' && !isAllowed(message.text())) { - collector.consoleErrors.push(message.text()); - } + if (message.type() !== 'error') return; + if (isAllowed(message.text())) return; + if (isExpectedMainDocumentStatusError(message, page)) return; + collector.consoleErrors.push(message.text()); }); page.on('pageerror', (error) => { From 6073af168a66f567c56dafed3a1faee4fe8e8f77 Mon Sep 17 00:00:00 2001 From: Brandonma21 Date: Thu, 17 Sep 2026 13:26:53 +0200 Subject: [PATCH 7/7] Fix false-positive suppression risk in browser-errors status check (LS-2940) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug fix - browser-errors.ts: replace the URL-comparison exemption with a main-frame navigation response check - Root cause: comparing the failed resource's URL to the page's own URL could wrongly suppress a real subresource error if that subresource happened to request the exact same URL as the page - Now tracks the actual main-frame navigation response's status via isNavigationRequest() + frame() === page.mainFrame(), reset on every navigation, so only that navigation's real status is exempt Context - Flagged by CodeRabbit on PR #60 as a minor hardening suggestion, unrelated to that PR's LS-4207 scope but fixed here since it was already part of the branch - Verified locally: special-routes.spec.ts passes on Chromium (including the 404-template test this logic targets), no regressions in page-structure.spec.ts (its one failure is pre-existing and unrelated — missing
landmark/heading hierarchy, not console-error handling) --- tests/helpers/browser-errors.ts | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/helpers/browser-errors.ts b/tests/helpers/browser-errors.ts index 7d43a7f..e8f8fdc 100644 --- a/tests/helpers/browser-errors.ts +++ b/tests/helpers/browser-errors.ts @@ -21,14 +21,13 @@ function isAllowed(message: string): boolean { // template. network-errors.ts already exempts the main-frame navigation // response for the same reason (a route can legitimately return a non-2xx // status); this mirrors that exemption for the console-side equivalent. -// For this specific browser-generated message, ConsoleMessage.location().url -// is the URL of the resource that failed — comparing it to the page's own -// current URL confirms it's the main document's own status, not a broken -// subresource (which would still be correctly caught). -const RESOURCE_STATUS_ERROR_PATTERN = /^Failed to load resource: the server responded with a status of \d+/; - -function isExpectedMainDocumentStatusError(message: import('@playwright/test').ConsoleMessage, page: Page): boolean { - return RESOURCE_STATUS_ERROR_PATTERN.test(message.text()) && message.location().url === page.url(); +// Tracked via the actual main-frame navigation response's status (reset on +// every navigation) rather than a URL string comparison — a subresource can +// legitimately request the same URL as the page, which a URL-based check +// would wrongly treat as the page's own status and swallow. +function isMainFrameNavigationResponse(response: import('@playwright/test').Response, page: Page): boolean { + const request = response.request(); + return request.isNavigationRequest() && request.frame() === page.mainFrame(); } /** @@ -37,11 +36,20 @@ function isExpectedMainDocumentStatusError(message: import('@playwright/test').C */ export function watchBrowserErrors(page: Page): BrowserErrorCollector { const collector: BrowserErrorCollector = { consoleErrors: [], pageErrors: [] }; + let expectedStatusErrorPrefix: string | null = null; + + page.on('response', (response) => { + if (!isMainFrameNavigationResponse(response, page)) return; + expectedStatusErrorPrefix = + response.status() >= 400 + ? `Failed to load resource: the server responded with a status of ${response.status()}` + : null; + }); page.on('console', (message) => { if (message.type() !== 'error') return; if (isAllowed(message.text())) return; - if (isExpectedMainDocumentStatusError(message, page)) return; + if (expectedStatusErrorPrefix && message.text().startsWith(expectedStatusErrorPrefix)) return; collector.consoleErrors.push(message.text()); });