revert: #3620603 Restore the ComponentTreeLoader subclass and unconditional service swap (!17) · Merge requests · project / canvas_override

Problem / Motivation

The intent of #3620603 was only ever to remove the need for canvas--2026-08-09--3567225--mr-948-minimal.patch. It was not to build a second editing mode. The work that landed (60c285f, c56b7b8) and the work still open (MR !16 (closed)) drifted into page-backed editing, which is a different feature with a different cost.

The patch is three lines:

-final class ComponentTreeLoader {
+class ComponentTreeLoader {
-    private readonly EntityFieldManagerInterface $entityFieldManager,
-    private readonly ModuleHandlerInterface $moduleHandler,
+    protected readonly EntityFieldManagerInterface $entityFieldManager,
+    protected readonly ModuleHandlerInterface $moduleHandler,

There is no in-module way around it:

ComponentTreeLoader has no interface.
Its service ID is its own FQCN, so a replacement is resolved by class.
Four consumers typehint the concrete class: ComponentTreeInputExtractor, ClientDataToEntityConverter, ContentTemplate, ApiLayoutController.

So any replacement must satisfy instanceof ComponentTreeLoader, which means it must extend it, which final forbids. The same-service-ID swap this module already uses for ComponentTreeMeetsRequirementsConstraintValidator works only because nothing typehints that concrete class.

Conclusion: CanvasOverrideComponentTreeLoader extends ComponentTreeLoader plus the service swap is the correct design. The missing piece is upstream in Canvas, not here.

Proposed resolution

Wholesale revert of the two #3620603 commits on 1.0.x, with no lines retained, restoring the subclass and the unconditional service swap.

Reverted:

c56b7b8 feat: #3620603 Edit per-content layouts on a backing canvas_page when Canvas is unpatched, which added CanvasOverridePageResolver, the CanvasRedirectController branch, the canvas_page_insert / canvas_page_update / node_delete sync hooks, the autowired service and the stale-container error message.
60c285f fix: #3620603 Skip the ComponentTreeLoader swap when Canvas ships it final, which added isComponentTreeLoaderExtendable(), the SWAPPED_PARAMETER, the "Per-content Canvas layout editing is unavailable" warning and '#disabled' => !$available on the enable checkbox.

Not touched, because they are not from this issue:

eebd1de fix: #3618040 Accept a NULL bundle in hook_entity_bundle_field_info_alter()
fcb822b feat: #3590567 Add a confirmation step before resetting a Canvas layout
fc9cdf2 Automated Project Update Bot fixes from run 12-864040

After the revert, CanvasOverrideServiceProvider.php, CanvasRedirectController.php and canvas_override.services.yml are byte-identical to 1.0.0-beta3, CanvasOverridePageResolver.php is gone, and the only difference left in CanvasOverrideHooks.php versus 1.0.0-beta3 is what #3618040 and #3590567 put there.

Known consequences

Both are accepted costs of reverting wholesale, not oversights.

1. Stock, unpatched Canvas fatals site-wide

60c285f's guard was the only thing preventing it, and the guard goes with the revert. Verified on Drupal 11.4.6 + drupal/canvas 1.10.1 with no patches:

Fatal error: Class Drupal\canvas_override\Storage\CanvasOverrideComponentTreeLoader
cannot extend final class Drupal\canvas\Storage\ComponentTreeLoader
in modules/contrib/canvas_override/src/Storage/CanvasOverrideComponentTreeLoader.php on line 29

Every page fatals, /user/login and /admin/modules included, and drush cannot bootstrap, so the module cannot be switched off through the UI. That is the original #3620603 symptom returning, and it is why the Canvas #3567225 patch is now a hard requirement rather than an optimisation.

2. The #3590567 reset POST is rejected until re-fixed

c56b7b8 also carried a CSRF fix for the reset local task, and a wholesale revert takes it with everything else. The task is back to generating its POST URL with bubbleable metadata:

$generated = Url::fromRoute('canvas_override.node.canvas.reset.do', ['node' => $node->id()])->toString(TRUE);
$cacheability->addCacheableDependency($generated);
$post_url = $generated->getGeneratedUrl();

RouteProcessorCsrf then emits a placeholder, and hook_menu_local_tasks_alter() has nowhere to register the lazy builder that would resolve it ($cacheability carries contexts, tags and max-age only). Verified on Varbase 11 after this revert: clicking Reset Canvas layout produces

403 POST /node/15/canvas/reset/confirm?token=...

with 'csrf_token' URL query argument is invalid. The no-JavaScript confirmation form at /node/{node}/canvas/reset is unaffected and still works.

This fix belongs to #3590567, not here, and should be re-landed there. Carrying it in a #3620603 revert is what made the original commit hard to reason about in the first place.

Verification

Both bases run the reverted tree from the same checkout.

Varbase 11 (varbase_project:11.0.x-dev, core 11.4.6, canvas 1.10.1, varbase-patches 11.0.x-dev, patched):

isComponentTreeLoaderExtendable() and SWAPPED_PARAMETER are gone; the swap is unconditional and the service resolves to Drupal\canvas_override\Storage\CanvasOverrideComponentTreeLoader.
Content type form: enable checkbox is enabled, no "unavailable" warning.
/node/15/canvas redirects to /canvas/editor/node/15 and the editor mounts with all five regions (canvas-side-menu, canvas-topbar, canvas-primary-panel, canvas-contextual-panel, canvas-editor-frame).
No canvas_page entities are created, canvas_override.node_pages stays empty, and CanvasOverridePageResolver no longer exists.
Reset via the HTMX task returns 403, as described above and expected.
varbase-e2e: 43 scenarios, 32 passed, 11 failed, up from 27 passed before the revert. All 11 remaining failures are Varbase environment mismatch in the suite, each confirmed with a control: the 403 page reads "You are not authorized to access this page." rather than "Access denied"; a plain node save on Varbase also renders no status message and no h1; and a user without toolbar access sees no local tasks at all, /node/N/edit included.
PHPUnit unit: 12 tests, 129 assertions, OK.

Stock Drupal 11 (core 11.4.6, canvas 1.10.1, unpatched): the fatal above. The e2e suite cannot run against it.

Local CI green-gate (gitlab-ci-local --file .gitlab-ci-local.yml): PASS cspell, PASS eslint, PASS varbase-e2e-dry-run, PASS pages.

Follow-ups, not in this MR

#3590567 — re-land the reset CSRF fix in its own issue: generate the POST URL without bubbleable metadata and add the session cache context, so a real token is embedded and the cached local-tasks array does not serve one user's token to everybody.
Canvas #3567225 / MR !948 — get the three lines committed upstream. That is the only real path to dropping the patch.
Canvas, new bug — ApiLayoutController::patch() passes $preview_entity as the host entity, which is NULL on /canvas/api/v0/layout/node/N, so any tree carrying entity-field prop sources 500s with A host entity is required to set entity field prop sources. while the UI reports "No changes". Varbase's own node.blog.full template hits this; it blocks the patched path too.
The module's permission tiers grant the tab but not the editor: on Varbase content_moderation denies with "No valid transitions exist for given account", on stock Drupal it is the missing edit canvas_page.
README.md documents /node/{node}/canvas/default, which no route provides (404).

User interface changes

The "Per-content Canvas layout editing is unavailable" warning and the disabled enable checkbox are removed from the content type form. The Canvas Override tab always opens the per-node editor; it no longer redirects to a backing canvas_page.

API changes

CanvasOverridePageResolver is removed. CanvasOverrideServiceProvider::isComponentTreeLoaderExtendable() and CanvasOverrideServiceProvider::SWAPPED_PARAMETER are removed. The canvas_page_insert, canvas_page_update and node_delete hook implementations are removed.

Data model changes

None. The canvas_override.node_pages key-value collection is no longer written; it was never part of config or schema.

Release notes snippet

Restore the ComponentTreeLoader subclass and its unconditional service swap; per-content Canvas layouts again require the Canvas #3567225 patch.

Checkpoints:

AI-Generated: Yes

revert: #3620603 Restore the ComponentTreeLoader subclass and unconditional service swap