Overview
- @lauriiii at #3555413-19: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`:

Images seem to introduce some interesting. It looks like we could map the image to "image" but there's also URI and root-relative URL. What's the difference between these different mappings? - @Wim Leers at #3555413-20: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`:
That is best explained by quoting the test coverage, which makes this very explicit:
'Silly image 🤡 → URI → Root-relative file URL' => 'ℹ︎␜entity:node:foo␝field_silly_image␞␟entity␜␜entity:file␝uri␞␟url', 'Silly image 🤡 → URI' => 'ℹ︎␜entity:node:foo␝field_silly_image␞␟entity␜␜entity:file␝uri␞␟value', "Silly image 🤡" => 'ℹ︎␜entity:node:foo␝field_silly_image␞␟src_with_alternate_widths',- The first ("URI → root-relative file URL"): the image field's reference to the File entity is followed, then the "url" property on the "uri" field on the File entity is retrieved.
- The second ("URI"): the image field's reference to the File entity is followed, then the "value" property on the "uri" field on the File entity is retrieved — aka the main property on that field, which is why there's one less level in the hierarchy for this one.
- The third ("just the image field itself"): the image field's
src_with_alternate_widthscomputed property that Canvas added.
You can only see all 3 if you have an SDC/code component prop with a very open-ended prop, because the second one returns a
public://URI. but, arguably, it does not make sense to expose "root-relative file URL" for an image field, either, becausesrc_with_alternate_widthsis likely preferable. Except … if you want to link to the original image, because then the query string is just noise.[…]
👆 Note the bold+underlined bit!
- @lauriii at #3555413-25: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`:
I think with the improved labels it would have made sense to me what was being offered there. FWIW, I was testing this with code components so maybe the prop type in code components should be setting stronger restrictions for the stream wrapper URL to not be offered?
- @Wim Leers at #3555413-26: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`:
The code component editor is correct, it's just coarse. I think we need to expand the code component editor UI (and the JSON schema for props that code components are allowed to have) to allow for optional granularity. This is especially true for links. Because the hyperlink is literally the foundation of the web, and it's also literally what a CMS is about: managing content and relations between them. So it's important that the author of a code component can actually specify what kind of link they expect. Because only then can Canvas offer relevant links!
This is what that UI currently looks like, annotated to convey what the coarse prop shapes are that you're allowed to pick:
Proposed resolution
Allow more granularity. See the change record that in August through September introduced the ability for highly structured approaches to restricting links. (Before then, it was a hacky pattern: …-based mess, aka regex hell 🧟).
1️⃣ Add a Link target prop setting in the code component editor UI
Which maps to contentMediaType. Offer 4 choices initially:
Anything
(DEFAULT! — plus, this is what all existing code components use. This corresponds to nocontentMediaTypebeing set in JSON Schema)Only images
(contentMediaType: image/*) → will result in only suggestions for image fields and image mediaOnly videos
(contentMediaType: video/*) → will result in only suggestions for video mediaOnly web pages
(contentMediaType: text/html) → will result in only suggestions for the host entity's URL (since #3545859: Add a `host-entity-url` prop source for linking to the host entity), entity reference fields (once #3555413: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()` lands) and link fields
Perhaps with a UI description along the lines of When this component renders an image, this setting allows ensuring only relevant links are offered.
2️⃣.A Add an optional Link protocol
prop setting in the code component editor UI
Which maps to contentMediaType. Offer 2 choices initially:
Any
(DEFAULT!)Web links
(setsx-allowed-schemes: [http, https])
Perhaps with a UI description along the lines of While most links are likely web links, there are more specific kinds of links too, such as linking into a chat application, video conferencing software or Drupal internals (so-called "stream wrappers").
2️⃣.B Alternatively, always restrict to HTTP(S) links
Alternatively, we could omit that second setting and just always restrict all "URL prop shapes" for code components to x-allowed-schemes: [http, https], and provide an update path to make it so.
User interface changes
TBD
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 3556144.patch | 1.79 KB | wim leers |
Comments
Comment #2
wim leersComment #3
wim leersHTML fixes.
Comment #4
wim leersHTML fix.
Comment #5
lauriiiI don't really see a strong reason to why we'd have to provide a ton of flexibility to configure this especially in the UI for creating props. That would be relevant if you're implementing business logic related to handling the link but implementing that in your component doesn't seem like the right place. Even the current settings don't really make sense in my mind and they are confusing; I had opened before #3520068: Allow mixing internal and external URLs. Since then we've changed the labels but I'm not sure I'd be able to explain the difference between the options still without looking into the implementation and referring to JSON Schema docs.
I think we should focus the default link prop to what's allowed by
hrefattribute, i.e. what's defined in https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/a#href. Based on that URLs must be absolute (start withhttp:,https:,mailto:,sms:, ortel:and have a hostname) or site relative (/page/123) or a hash (#something).Comment #6
lauriiiComment #7
wim leersHere's the config schema changes we'd need.
Comment #8
wim leersI disagree that "this component prop needs an image URL" is "business logic". It's component expectations. Which is what you identified at #3555413-19: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`, and which is what prompted this issue.
I've already explained this in the issue summary in detail.
To be fair, I am approaching the full problem space here. Just always setting
x-allowed-schemes: [http, https]should be sufficient to solve what you raised at #3555413-19: Allow linking to referenced entities: add `url` property to `EntityReferenceItem::propertyDefinitions()`. That's what I proposed, too: 2️⃣.B.Comment #9
lauriiiI don't see "this component prop needs an image URL" as a common use case – >99% of the time you would just use image upload / media library for that use case. I don't think this is something we should support in the code editor. I'm not sure what you're linking to because it results in a 404.
x-allowed-schemes: [http, https]doesn't seem sufficient because as I pointed out, there are many valid use cases for links that are using protocols other than those two.