Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert links to their internal link representation if they exist #4984

Open
herbdool opened this issue Mar 6, 2021 · 9 comments · May be fixed by backdrop/backdrop#3569
Open

Convert links to their internal link representation if they exist #4984

herbdool opened this issue Mar 6, 2021 · 9 comments · May be fixed by backdrop/backdrop#3569

Comments

@herbdool
Copy link

herbdool commented Mar 6, 2021

Description of the bug

When adding an internal link to a Link field, and with language prefixes enabled, I'm getting fr/fr/path. It should be only adding it once.

Enable Link, Language, Content Translation. As two languages and set it to negotiate with prefixes. Enable translation on the page content type. Add some dummy content in both languages. Add a link field to any content type and add a node which references a page in another language.

Possible solution

Convert links to their internal link representation if they exist. https://www.drupal.org/project/link/issues/836710 fixes this by providing a new option: convert the URL to an internal system path. An alias gets turned into node/1 for example.

@indigoxela
Copy link
Member

indigoxela commented Mar 7, 2021

@herbdool isn't that a duplicate of #4919?

FTR: the initial PR for #4630 (closed) also tried to solve that.

UPDATE: wait, this is about the link field, not the filter dialog, right?

@herbdool
Copy link
Author

herbdool commented Mar 7, 2021

This is about the field so it's not a duplicate of that one. It's got its own weirdness.

@herbdool
Copy link
Author

I wonder if this is fixed in d7 now with https://www.drupal.org/files/issues/2019-12-12/link-n836710-44.patch which attempts to convert the link to its internal path (eg. node/1).

@herbdool
Copy link
Author

@indigoxela I've got a PR for this.

@indigoxela
Copy link
Member

@herbdool Cool, many thanks!

But... the scope of this issue seems to have evolved. It might make sense to update the description and - most of all - the issue title to prevent confusion.

This is the D issue, right? https://www.drupal.org/project/link/issues/836710

Is this covered by any of our meta issues? We have several for the Link module, and I lost track.

@herbdool
Copy link
Author

Yes, it's that issue.

It's not covered by the Link Meta issues. It's been committed in d7 but not in a release yet (would be 7.x-1.8). I don't think it's scope has evolved so much because it does fix this specific bug by providing a new option. It also helps that it helps keep it in line with the d7 module.

@indigoxela
Copy link
Member

Evolved means, that it adds another field setting form item (UI change) and provides features beyond the prefix duplication bugfix.

The issue title and description should cover that.

I'll try to test this as soon as possible (not today).

@herbdool herbdool changed the title Language prefixes are being repeated in Link module fields Convert links to their internal link representation if they exist Mar 16, 2021
@indigoxela
Copy link
Member

indigoxela commented Mar 17, 2021

I did some testing. Bad news, the output is still wrong for multilingual.

This is how I tested:

  • Enabled and configured language, locale and translation modules
  • Added a language
  • Added a link field to the post content type, set it to "Convert local aliases"
  • Added content of that type with a link to "posts/your-first-post" (via autocompletion)
  • Saved the node
  • Opened the node form again: the URL in the form field was displayed as "node/1" 👍
  • The actual url in node display is /de/de/posts/your-first-post or /en/en/posts/your-first-post, though

So the new feature works, internal paths get handled as expected, but the prefix problem isn't solved by it yet.

A screenshot of the newly added setting in the advanced options of link field:

convert-aliases

@indigoxela
Copy link
Member

indigoxela commented Mar 17, 2021

The problem seems to be that in function _link_sanitize() the "url" variable has already been run through url() and in the theme function this happens again via l() - which also calls url().

I see, the Drupal community also realized: https://www.drupal.org/project/link/issues/2428185

Was fixed with Link D7 version 1.7, so we don't have a meta covering that fix yet, I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants