Fixed
Project:
Canvas Field Component
Version:
1.0.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
28 Apr 2026 at 19:34 UTC
Updated:
29 Apr 2026 at 13:50 UTC
Jump to comment: Most recent
Comments
Comment #3
mandclu commentedComment #4
mandclu commentedFeedback from Codex:
I’d mark the MR Needs work, mostly for access-control hardening. The HTMX direction is good, and the core API usage is broadly right.
What Looks Good
The overall replacement of the Redux-polling custom JS with Drupal core HTMX is a solid improvement.
Htmx::applyTo()automatically attachescore/drupal.htmx, so removingcanvas_field_component.libraries.ymlandjs/component-form-refresh.jsmakes sense. HTMX also includes the enclosing form values for non-GET requests, so the form-context POST strategy is reasonable.MR pipeline is green: MR !3.
Main Concern
The new route is too permissive:
Because
FormatterSectionFormreads arbitrary postedcanvas_component_propsand uses those to load field definitions / formatter settings, anyone withaccess contentcould POST entity type, bundle, field name, and formatter IDs to get form fragments back. That permission is often available to anonymous users. This endpoint should use the same access model as editing the Canvas template/component instance, or a custom access checker that validates the actual editable template/context. At minimum, it should not beaccess content.Relevant code:
canvas_field_component.routing.yml:src/Form/FormatterSectionForm.php:46-83Secondary Concerns
The MR includes an unrelated rendering-path change: replacing
FieldItemListInterface::view()with a clonedEntityViewDisplaypath inbuildFieldRenderArray(). That may be a good fix, especially for third-party formatter settings needing display context, but it is behaviorally broader than “replace custom form refresh with HTMX.” I’d either split it into a separate issue/MR or explicitly document why the HTMX change requires it.There are no test changes in the MR. I’d want at least coverage that:
- the field select has the expected HTMX attributes;
- the formatter section POST returns only the formatter section;
- the route denies users who cannot edit/configure the Canvas template;
- formatter changes still preserve/update settings correctly after swap.
Suggested Maintainer Feedback
“Approach looks promising and the Drupal core HTMX usage is appropriate. Before merge, please tighten the HTMX route access so it matches Canvas template editing permissions rather than `access content`, and add coverage for the fragment endpoint. Also consider splitting the
buildFieldRenderArray()rendering-context change into a separate issue unless it is required for this one.”Sources checked: Drupal `Htmx::applyTo()` API, HTMX request parameter docs.
Comment #5
mandclu commentedComment #7
mandclu commented