Problem/Motivation
After an in depth irc discussion with Fabianx just now (and with many others at DrupalCon), opening a new meta issue to document how I understand the various use cases we're trying to fix with url/l and menu link storage.
We need to flesh out the following use cases, then how they'll be supported.
1. Storing as user-entered data, then later rendering as a link to the entity, an entity reference (main use case for user-entered menu links) with 100% coupling between the link and the entity.
2. Storing as user-entered data, a menu link pointing to a path - the path may or may not resolve to a valid route and it may or may not be an alias - where it does resolve it will get outbound processed, otherwise we assume it was entered for a good reason. We think this is OK since it's impossible to determine which route a path resolves to or if it does at all except at runtime. If I have two routes at the same path, which do you pick? Worst case is you have valid routes on the site pointing to a 404, better than deleting for no reason.
2a. Generating a link from user entered path data with tokens - so the link generation can only be done at run-time. (Display Suite use case)
3. Generating a link in code to a route provided by a specific module with the expectation it will stay there (generateFromRoute())
4. Generating a link to a path that is module-agnostic - for example supporting /blog - which could be implemented by different modules including views, or could point to a path where two different routes are valid depending on priority. This is a real use-case for generateFromPath() which we just removed.
4a. Current use case in core: Link to a dynamic views route. Simple with paths (/events), but difficult with routes as they are not canonical.
5. Updating paths for routes in a module, when the module is already installed and supported on live sites. I think this is a migration path problem since those links will break in the browser too (i.e. not supported in core, could be supported by 8-8 migrate).
Proposed resolution
Provide the following API:
- Url::fromRoute() - Generate a link specific to a module providing a certain route name
- Url::fromPath() - For very specific use cases with a _very clear_ definition of when it should be used (e.g. module agnostic, dynamic path) and when its better to use a real content link (use the entity) or a route.
- Url::fromUri() - External urls, base://, etc.
- Url::fromResource() - Replace / Deprecate file_get_url() to support JS, CSS, Images via CDN
Provide the same API for Twig land, it already has {{ url_from_path() }}, but unsure if that function is deprecated or not atm.
- Split up the use case to link to a content entity with the use case to link to a path, those are two different things
- Remove automatic route resolving for non-content, path links - it cannot be done at path entering time as the context needs to be known at run time, but then it _can_ be done.
Possibly fix cases where context for a generateFromPath() needs to be resolved based on the current request object to find the correct route - if not done so already.
Remaining tasks
- Discuss
- Make Url::fromPath() / generateFromPath() performant
- Implement plan
User interface changes
- Likely in sub issues: Split up Content and Internal Link to two different options or tabs, e.g.:
Select Link Type:
( x ) Content Link
( ) Internal Link
( ) External Link
[ ( ) Resource - optional, possibly contrib ]
And:
- For content link show a nice entity reference browser, together with a paste field to find the right content by pasting a URL, which then resolves to an entity reference / route reference - depending on implementation.
- For internal link have what we have now, but possibly store as system.user_entered_path
- For external link, the external widget as right now already done.
This is _only_ an example showing it is possible to decouple this, any serious discussion belongs to sub issues with UX team review, etc.
API changes
- Undeprecate generateFromPath()
- Provide Url::fromPath(), but encourage its use only for very specific use cases
- Provide Url::fromResource()
- Undeprecate {{ url_from_path }} in Twig if needed and provide the remaining newly added functions
Comments
Comment #1
fabianx commentedComment #2
fabianx commentedComment #3
aspilicious commentedI agree with the summary, tomorrow I'll groep the ported ds code to see if I have other cases that arent covered het.
Comment #4
larowlanWorth noting #type path. User enters a path but is validated and stored as route name and parameters pair. No uses in core, plans to use in contact module for configurable submission redirects
Comment #5
dawehnerHaving the abstraction of an URL certainly allows us to easily expand the coverage for paths, which also runs through path processing.
Comment #6
fabianx commentedComment #7
tim.plunkettAs far as the changes to Url, I like what is proposed above. #2347465: Convert all instances of #type link/links to convert to use routes will make it much easier to find all of the edge-cases, since we've found them already.
Comment #8
dawehnerToday I ran into the following usecase: Create a menu with links to all english posts and all german posts, there is simply no way to do that yet in HEAD.
Comment #9
catch@dawehner I'm not clear which of the following situations your post means:
1. A single menu that has specific references to German and English translations as separate entries - these links should not be affected by the current user's language but be explicitly tied to a translation. So if you're viewing the site in French, they still point to German and English versions of the entities.
2. Two menus, one of which links to English entities, the other links to German entities, with the same behaviour as #1 in that those links are tied to a specific language. (The menu's visibility overall might be tied to language though).
3. A single menu that has references to entities that when rendered points to the correct translation via language negotiation.
For #1 or #2 that sounds like an 'entity translation reference' where you choose an explicit language as well as entity ID. I think it's a valid use case, but one that fits into the overall framework we arrived at here without requiring a rethink of the existing use-cases (i.e. if we have a new field type and wire it up to menu link entities or a new plugin type, it ought to be OK?).
For #3 I'd expect that to work in HEAD already...
Comment #10
fabianx commented#1 and #2 would be pretty simple to do with linking to paths - if we allowed it until the Entity Reference Translation tool is available.
Comment #11
pwolanin commentedSo, I'm not sure the issue summary is correct for point#4 - as I understand it, when overriding /blog Views actually replaces the route definition while leaving the original route name.
I expect if we have a fromPath() method it will be widely abused by contrib, regardless of documentation.
@catch - regarding links to English or German, from discussions with dawehner, this is basically a bug that we don't have a way to set or respect the langcode for a link.
e.g. if I want to link specifically to the German version of a post, that langcode needs to taken perhaps from a /de/ prefix on the user-entered path or as an option in the form and stored with the link. That seems like a bug or API-gap in core.
Comment #12
fabianx commented#11: The problem is that paths are the abstraction layer and routes are the implementation, because we have a 1-n relationship between paths and routes.
So any user entered paths needs to be resolved at run time, because we need the context of what the user meant.
e.g. Module X provides /blog for spanish, Module Y provides /blog for english (but with different access rules)
User links to /blog - we cannot determine at user entering time, which route to use as it depends on the context, which we only have at run time.
Even in my own code I cannot reliably link to that path at the moment. Which route do I use?
Already in D7 with hook_url_outbound_alter() you could move 'paths' around and change the underlying "route" / menu entry too, but you would always need to update all content linking to that, so creating a migration problem.
And most people used aliases for that.
So while you could already in practice change "routes" around in D7, this was seldom done, so I expect the use case for actually moving paths around rather low and as you need a content migration strategy anyway, you could also update the code at the same time to match the new paths.
Routes are probably superior when you want to target a specific implementation, but less if you need the abstraction layer of the path or even alias system.
Comment #13
dawehnerAdding a related issue which should improve the performance in case we start storing the path again, for all entity types.
Comment #14
chx commentedSo we had a very long discussion with dawehner and effulgentsia today -- with a very surprising ending. Not sure whether this is the best issue to post this but it needs to be posted somewhere.
The user might want to link to the entity of type node, number 1 which has an alias
content/this-is-a-node-titleby entering just that and the expectation is that if the alias changes then the link changes as well. We call this a route reference.The user might want to link to the
about-uspage and expect it not to break even if the page under the path changes -- it might point to a new node or page manager or whatever. We call this a path reference. One of the most important results we have arrived to that path references are very literal and outbound processors should not run over them, not even language. If you have a need, then the field is multilingual and you can translate it. Even if you point a path reference to an entity which has translations and the translations have aliases you won't get those automated -- that's the price for path references. The path reference is literal. It is possible even that the "about us" page say in English is a very fancy page manager page and in another language it's just a simple description. This case is covered as well. When creating the link we need to try to route and access check base:// links. We will need to cache the results but access check objects are already carrying caching information -- thanks Wim :)We need to allow the user to pick which one he wants. It is completely impossible to determine this automatically.
Surprisingly, by accident, HEAD already has support for both with route reference being the default and path reference made available by
base://. So the link field description could say "If node/1 has an alias about-us then entering about-us and changing the alias will point to the new alias of node/1. Linking to base://about-us will always point to about-us. base:// can also be used to link paths on the same host as Drupal but not handled by Drupal."Comment #15
fabianx commentedThat is good and as long as custom code can also use
Link::createFromUrl(Url::fromUri('base://some-path'))
which is then access checked, this is totally fine.
My only ask (and aspilicious had the same idea) is to make this API more accessible, so that
Url::fromPath('something') would internally correspond to Url::fromUri('base://something')
And I think there is the whole mis-understanding. I never wanted to support putting in node/1 then have language neg, alias support, etc. be supported (which would be a different url generator), no Url::fromPath() should really be just for what base:// does now (plus the access checks), but it feels less 'hacky', but more legitimate.
We still need to solve the issue of having persistent dynamic routes for views pages or panels, but we talked some in IRC today with dawehner and came up with the idea that a UUID could be supported, which means a dynamic route could be transferred from a view to a panel - without having to migrate all the content.
So a canonical identifier for dynamic routes (and paths).
Also the UX of adding base:// will need a UX re-review of the really nice link interface as currently one could only put in base:// as 'external link', which feels kinda strange.
Comment #16
catchComment #17
chx commentedComment #18
catchIsn't that an entity reference?
I think we should be able to do the following in core, and contrib could handle adding extra options:
1. Link to path - exactly what chx/effulgentsia said with path reference, doesn't get anything applied except access checks.
2. Link to entity - add entity reference menu link type and that sticks to the entity.
If you browse for an entity when creating the link (or add the link from an entity form), you get an entity reference link.
If you specify a path when you create a link, you get a path.
Also we've had usability issues open for an entity reference browser when creating link for a long time, so it should even be an improvement there.
What core has and currently breaks is the 'route reference' i.e. explicitly linking to an explicit route name provided by a module. Generally I don't think this makes sense with storing routes from user-entered data - the route is entirely an implementation detail and that concept shouldn't be exposed anywhere, whereas 'this path' or 'this entity' is something that is a central concept in Drupal site building - that's just urls and content.
PHP code can still link to routes - that isn't stored anywhere except the code.
Comment #19
xjmComment #20
xjmComment #21
andypostI'd like to point #2010132: Canonical taxonomy term link for forum vocabulary is broken there's 2 ways to build a aliased path for entities:
1)
uri_callback- the most performat but replaces a route name2) outbound processor - looks a right way because use more runtime context to determine a path, but slow
So time when route becomes a link should be taken into account too.
Comment #22
webchickOk, based on a mega-call today with various people outlined at these meeting minutes, moved the use cases we determined to a handbook page at https://www.drupal.org/node/2407497 since they'll be useful for any future changes. So the "determine" part is figured out.
The "support" will be defined in sub-issues of #2407505: [meta] Finalize the menu links (and other user-entered paths) system.
I believe that this issue can now be closed.
Comment #23
xjm