We have the route
canvas.boot.entity:
path: '/canvas/editor/{entity_type}/{entity}'
defaults:
_controller: 'Drupal\canvas\Controller\CanvasController'
_title: 'Drupal Canvas'
requirements:
_canvas_authentication_required: TRUE
_canvas_component_tree_edit_access: TRUE
_entity_access: 'entity.update'
options:
parameters:
entity:
type: entity:{entity_type}
_format: 'json'
But in \Drupal\canvas\PathProcessor\CanvasPathProcessor::processInbound we have
if (str_starts_with($path, '/canvas/') && !str_starts_with($path, '/canvas/api')) {
return '/canvas';
}So at least in \Drupal\canvas\Controller\CanvasController::__invoke we will never call if from this route
Is this intentional?
I created an MR ensure I am correct and only \Drupal\Tests\canvas\Kernel\LibraryInfoAlterTest::testTransformMounting failed because it calls the controller directly.
Is canvas.boot.entity only there so we make links to the path? If so why do have logic around $entity_type and $entity in \Drupal\canvas\Controller\CanvasController::__invoke because these will always be NULL?
Not sure
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | Screenshot 2026-01-22 at 2.40.22 PM.png | 114.5 KB | wim leers |
| #10 | Screenshot 2026-01-15 at 8.23.34 PM.png | 139.16 KB | wim leers |
| #10 | Screenshot 2026-01-15 at 8.21.51 PM.png | 619.48 KB | wim leers |
| #10 | Screenshot 2026-01-15 at 8.20.02 PM.png | 1.16 MB | wim leers |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
tedbowComment #4
tedbowComment #5
tedbowComment #6
tedbowSetting to Needs Review for someone to double check my assumptions
Comment #7
wim leersGrepping the codebase for that route name indicates
\Drupal\canvas\Controller\ApiContentControllers::getEntityOperations()uses this.#3529836: Enable starting with an empty XB UI (so without first having to create an entity with a component tree) introduced this. I think you'll find answers there. That's where I'd start looking.
Comment #8
tedbowBack to @wimleers to double check this https://git.drupalcode.org/project/canvas/-/merge_requests/466#note_657291
Comment #9
wim leersQ for Ted: https://git.drupalcode.org/project/canvas/-/merge_requests/466/diffs#not...
A for Ted:
experience_builder.boot.entityroute would have been matched because/xb/node/42is retained/canvas/api/somethingis retained, but/canvas/node/42is transformed to just/canvas!And … guess what … I literally called this out as a concern over at https://git.drupalcode.org/project/canvas/-/merge_requests/4/diffs#note_..., which was merged without my approval 😅
The commit that introduced this: https://git.drupalcode.org/project/canvas/-/merge_requests/4/diffs?commi...
⇒ AFAICT this was an unintentional side effect. Needs further investigation to understand the consequences.
Comment #10
wim leersThis was almost a security vulnerability, because due to #3502887: Prepare for avoiding full page reloads: move entity type and ID from base path and into routing parameters, the wrong route is matched:
Which causes an fewer route requirements to be checked:
So, if for the current user those route requirements are checked, so just this one:
Then the resulting call to
\Drupal\canvas\Access\CanvasUiAccessCheck::access()allows access, then I'm getting a 200, not a 403! 😱However, thanks to it then booting the Canvas UI, and the Canvas UI trying to fetch the right info, it ends up not revealing anything:
Comment #11
jessebaker commentedHopefully I can give a bit more history/context as to why this change came about. Then someone with more understanding of the Drupal side of things can weigh in on if there is a better approach.
At the start the Canvas React app could only be accessed by going to a URL for a specific entity that was to be edited.
As Canvas grew that made less sense - you might want to navigate straight to a Code Component in the Code Editor for instance. And then maybe we want
/canvasto allow people to see the UI without any task in mind and then pick what they want to do from the menu.To enable this Canvas React app is a Single Page Application - all the routing and loading of data is handled by the client based on the URL route after
/canvas.The intent then is that whenever a user goes to any URL that starts
/canvas(e.g./canvas,/canvas/editor/,/canvas/editor/canvas_page/1,canvas/doesnt_exist) they are served essentially the same practically blank document with the React App JS assets. The React app then initialises, examines the route parameters and decides which data to fetch (so if the url is /canvas/editor/canvas_page/1 it will immediately fetch the layout/model for that entity from the API).Essentially the desired outcome is that Drupal ignores everything after the
/canvaspart and no matter what comes next in the URL path Drupal always serves the same page.One wrinkle in the above that exists at the moment is that we have not yet fully solved the technical challenge of navigating from one edited entity to another without forcing the whole page to reload. Certain parts of the app still assume/rely on data being present on page load rather than being dynamically fetched. This is being addressed in #3538104: Add seamless SPA navigation between editing different pages/content.
Comment #12
penyaskitoOptions:
a) remove
canvas.boot.entity.b) If we use that as a "helper" for e.g. generating links, let's just ensure it has the same requirements than canvas.boot.empty.
a) Keeping it is still useful for e.g. generating links from the admin UI in the backend.
b) Then I was thinking that
canvas.boot.entityshould have the same requirements thancanvas.boot.empty. But as Drupal already checks access before rendering a link, this would require boilerplate code when we want to have a link at the backend.This is not a problem I've seen before, as we traditionally don't often have SPAs inside Drupal. My suggestion is:
a) We don't remove
canvas.boot.entityb) We add very explicit comments saying that this is not run when the actual page is visited as the routing actually happens client-side. But still we want to know the route server-side and check if the user will have access to it in the UI.
c) We add a cross-referenced comment with @see in routing.yml and wherever those permissions are checked client-side, pointing to each other, ensuring that we keep those in sync.
Comment #13
penyaskitoRelated: #3568238: Create route for content template . If the outcome of this is NOT what I described in #12, then #3568238: Create route for content template should be a Won't fix. Because it will introduce the same problem!
Comment #14
vishalkhode commented@penyaskito: This is the same reason we want to add a new route for Content Template. I agree we should keep the existing route unchanged and add comments to clearly document why this route exists and the rationale behind it.
Comment #15
wim leersAgreed with #12 + #13 + #14.
I'm not yet convinced that this is long-term desirable:
… nor that it is required for #3502887: Prepare for avoiding full page reloads: move entity type and ID from base path and into routing parameters or #3538104: Add seamless SPA navigation between editing different pages/content. I think that making Drupal ignore everything after
/canvaswas the sane/simple choice to keep the Canvas JS codebase as simple as possible for now.While I understand that this is the intended behavior on the client side, I think that this misses an important optimization opportunity: the
canvas.boot.entityroute (/canvas/editor/{entity_type}/{entity}) could pre-load data that otherwise requires additional requests.Loading
/canvas/editor/canvas_page/1currently triggers the following API requests:Those requests to
/canvas/api/v0/layout/canvas_page/1and/canvas/api/v0/form/content-entity/canvas_page/1/defaultare the ones that are specific to "the entity being booted into Canvas". They could be preloaded, server-side, and embedded in the first response. They're also the slowest. (And that's for the simplest content entity in existence, with an empty component tree, with zero network latency.)IOW: I think that eventually, we'll actually want to make the client a bit more complex, to face the reality that Canvas would be too slow to boot (especially on high-latency networks) to achieve the desired front-end performance.
We've not yet done any meaningful performance optimizations on either the server or client side. Some things here and there, yes. But the focus has been, and still is, on feature expansion. Not performance or polish. That will change eventually.
IOW: I think making the server ignore everything after
/canvasis the right call right now to prioritize iteration and features, and avoid premature optimization. But I doubt it we will want to keep it like this.So, IMHO, #12's a+b+c make sense, and I would add d:
which would have answered my question on the original MR: https://git.drupalcode.org/project/canvas/-/merge_requests/4/diffs#note_...
Comment #16
jessebaker commentedYour comment there makes sense and is in line with what I’m thinking.
Once #3538104: Add seamless SPA navigation between editing different pages/content lands then there won’t be additional page loads being asked of Drupal after the initial one because all the subsequent navigation within Canvas will be handled purely client side via fetching the data.
The ideal of preloading data would be a nice optimisation in the longer term however it's going to be an optimisation for that initial page load. I think we’d be better off putting our efforts into optimising that new way of moving between pages once #3538104 lands.
Having a nice generic way of preloading various data based on the initial route the user lands on would be cool - be that a page or a template to edit or a component in the code editor.