Closed (fixed)
Project:
Link
Version:
7.x-1.x-dev
Component:
Miscellaneous
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
19 Nov 2019 at 21:07 UTC
Updated:
5 Jan 2022 at 11:25 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
damienmckennaWhat output settings are you using for that field?
Comment #3
tcnolan7 commentedHi Damien,
It's set to "URL, as plain text"
Terry
Comment #4
ron collins commentedSeems like we might be having the same problem. We hide the link in a view (the link field has the formatter "Title, as link (default)") and then we rewrite the title field with the token [field_link].
Every URL with a query param is cut off. Downgrading to 1.6 fixes.
I won't change it but I'd suggest this is higher than normal priority and probably a quick fix.
Comment #5
sashken2 commentedI have same problem too.
I use "URL with the protocol removed" formatter.
Comment #6
damienmckennaSeems like we need to improve the test coverage to make sure this works as intended.
Comment #7
timwright_ commentedFollowing this issue because we are experiencing the same problem. What I have noticed is the external link is truncated for a "icon menu" paragraph bundle. Using the nice menus module I can set the external link and it is not truncated. Confirmed downgrade to 1.6 resolves the issue.
Comment #8
silverham commentedThis issue seems to be introduced by #2428185: Language prefix and relative links broken
Comment #9
silverham commentedAttached patch reverts #2428185: Language prefix and relative links broken as a hotfix for now. Needs better fix for the original issue.
Comment #10
summit commentedHi, Thanks for this great patch as a hotfix! It was very important for me.
I use label as link, with label as title, and then I rewrite it to
greetings, Martijn
Comment #11
silverham commentedThe problem seems to be that us developers were never mean to use the
$item['url']directly as is.We should of used
l()orurl()in the template / preprocess if doing any custom html chnages.A quick mock up could be to use
hook_preprocess_field()to get back old way and and render the$item['url']manually before any templates. (see below).Future work however should take these into account. @see
theme_link_formatter_link_default()Comment #12
la558 commented@Ron Collins (post #4) and @timwright_ (post #7) how did you downgrade to 1.6?
I have valuable information on those fields, so uninstalling/re-installing this module is not an option.
Thanks
Laura
Comment #13
timwright_ commented@La558 Unfortunately there is no "official" way to downgrade. We were able to use a backup to revert the update.
The most useful I can find: https://www.drupal.org/forum/support/upgrading-drupal/2010-10-28/closed-...
Comment #14
ron collins commented@La558
I used drush to downgrade:
drush dl link-7.x-1.6There are no database updates between versions so that should be safe enough. Clear your caches after.
If you don't have drush available, you could also download from here:
https://www.drupal.org/project/link/releases/7.x-1.6Comment #15
la558 commentedThank you for your comments @Ron Collins and @timwright_
Since the upgrading it to 1.7 was my last commit I reverted it.
And as the two of you experienced, going back to 1.6 resolves the issue.
Comment #16
damienmckennaLet's see what the testbot says.
Comment #17
damienmckennaWe still need test coverage to make sure the correct expectations.
Comment #18
coufu commentedHaving issues here too with URL fragments getting stripped. Rolling back to 1.6 in the meantime.
Comment #19
adriancotter commentedI have this issue to using tokens in Display Suite.
Comment #20
damienmckennaThere's already test coverage for this - see LinkUiTest::testLinkCreate().
I suspect the best solution would be to adjust the overridden output via a preprocess function to add the extra class attributes.
Comment #21
damienmckennaA slight tidying of that test.
Comment #22
arcataroger commented@silverham, thank you!! Using the url() function in my template fixed it. Is this documented anywhere? I looked through all the docs for the link module I could find, and it wasn't mentioned as far as I could tell. Is this a best practice for Drupal in general?
Comment #23
wylbur commentedApplying the patch to the 7.x-1.7, and this resolved issues with truncated links.
Thanks for quick action on this patch!
Comment #24
damienmckennaWe don't want to roll back the existing changes, but we could work on improving handling for this scenario. To do that we need to start with documenting how to trigger this bug, write tests for it, and then work out how to fix the underlying problem.
Can someone please help by documenting how to configure the site in such a way that it triggers this problem? Thank you.
Comment #25
damienmckennaThe minor test changes were committed in #3110061: Tidy up test files, so we can focus this on fixing the actual problem.
Comment #26
Michael-IDA commented{delete, wrong issue queue!}
Comment #27
jvogt commentedI was able to trigger the issue in Views, but not in my custom template. Here's my config.
Field
Custom link field (field_file_image_custom_link) on the Image file entity (using File entity module).
Field config
The key setting seems to be the Link Title. The issue is triggered when No Title is selected, but not with Static Title. I haven't tested the other options.
The rest of the field config is as follows: (Most of it is the default settings)
Test field value
https://www.google.com/search?client=firefox-b-1-d&q=drupal%20link
View
A table view listing Image file entities. Add a column for the custom link field and choose Title, as link (default) or URL, as link as the Formatter.
Output
The view column shows the truncated URL--https://www.google.com/search--as the title, but the full URL as the href. When I do the same with 7.x-1.6, both the title and the href are the full URL.
Custom template
I was unable to trigger the issue in my custom template (file--image.tpl.php) using entity_metadata_wrapper ($file_wrapper->field_file_image_custom_link->value()['url']).
Comment #28
damienmckennaCould someone please build a small view that shows the problem and upload it here as file? That could give us something to work against to make sure we work out a fix for this problem.
Comment #29
bondjimbond commentedYou don't even need a view; any node with a Link field and an affected URL will demonstrate the problem. View the node with the default display mode and you'll see that everything after the ? is cut off.
Example data: use the URL https://search.ebscohost.com/login.aspx?authtype=ip%2Curl%2Cuid&profile=...
What displays: https://search.ebscohost.com/login.aspx
Comment #30
damienmckennaCould someone please build a small Feature that includes the necessary configuration to show the problem? We can then wrap that in a test. Thank you.
Comment #31
bondjimbond commentedFeature attached. Just use the sample link
https://search.ebscohost.com/login.aspx?authtype=ip%2Curl%2Cuid&profile=ehost&defaultdb=psyhwhen creating a node in the "Test content type", then view the node -- you'll see all the link parameters are cut off.This is done on an extremely vanilla Drupal with no custom templates using the Bartik theme.
Comment #32
niles38 commentedWe are downgrading to 7.x-1.6 as well. This issue should be addressed ASAP.
Comment #33
skrossa commentedHaving what seems like the same or similar truncation issue using tokens for a Link field in a Display Suite custom code field (in a Paragraph item).
For a link field named "File Source", with specific data
Title="Entered Title"
URL="https://example.com?parameters"
the following tokens get these results:
Comment #34
danbarron commentedI may be way off base here since this the first time I've looked at Link's code, and it's not clear to me what the actual intention is here.
In _link_sanitize, the following change was made in commit a1870f1efcd2a0c170243fe0fcc12c8a0b9a9e41 starting at line 584:
Before: appears to just update the ['url'] property with the full (sanitized) URL built from its parts.
After: replaces the full $item object. It overwrites the original ['url'] with the base URL from the parsed parts and never actually returns the full sanitized URL.
Is the idea here to go ahead and return the parts, since they're being calculated anyway and might be of some use? Then I would recommend adding a ['base_url'] property:
If returning the parts is not wanted I think the code could be reverted to what it was.
I also wonder if ['html'] should be ['https'] since there is no 'html' option for url().
If I'm on the right track here I could submit a patch.
Comment #35
silverham commentedThis usually means
l()won'tcheck_plain()the title of a link if 'html' is set to true. Great work on trying to fix issue though. :-)Issue summary so far:
This still seems as issue for some users....
URL inside link tag is OK.
title generated from url (title optional/no input): url is cut off. - NOT OK
URL from token_replace() - NOT OK.
Tests for 7.x-1.7:
URL: https://search.ebscohost.com/login.aspx?authtype=ip%2Curl%2Cuid&profile=...
token_replace() e.g. in a link template file.
Views showing a link from node (same as result below)
if rendering a node view:
(based off #31 feature)
Title, as link (default) - URL is OK, title has parameters url cut off (if no title entered of course, if same url is entered as title it is OK)
- result: title: https://search.ebscohost.com/login.aspx Link : OK
Title, as plain text - title has parameters url cut off (if no title entered of course)
- result: title: https://search.ebscohost.com/login.aspx
Host, as plain text - Ok - Not applicable.
- result: title: search.ebscohost.com
URL, as link - URL is OK, title has parameters url cut off (if no title entered of course
- result: title: https://search.ebscohost.com/login.aspx Link : OK
URL, as plain text
- result: title is OK
URL, absolute
- result: title is OK
Domain, as link - Ok
- result: title: search.ebscohost.com Link: OK
URL with the protocol removed
- result: title: search.ebscohost.com/login.aspx Link : OK
Short, as link with title "Link"
- result: title: "Link" - Link: OK
Label, as link with label as title
- result: title: "Link" - Link: OK
Separate title and URL
title: https://search.ebscohost.com/login.aspx & Link: (title https://search.ebscohost.com/login.aspx) - url is OK
Comment #36
alpe89 commentedDowngrading to 1.6 works as #14 said
Comment #37
BrianLewisDesign commentedDowngrading back to 1.6 per #14 fixed the query string, but broke some other links and stripped the href. So no good.
Patch #21 did not fix URL, the question mark and query string are still stripped off. It was just a test file that was patched. @DamienMcKinna
Code suggestion #34 did work on, effectively the same as patching just the query string backwards to 1.6. @danbarron
Link module 7-x.1.7 - link.module file - line 591
Replace the full if (!empty($url_parts['url'])) code block with this:
Please fix this module ASAP. Working links are important. :)
Comment #38
damienmckennaFinally ran into this on a client project.
Comment #39
damienmckennaIn my specific use case it turned out someone was grabbing the raw 'url' element instead of passing it through url(), so all of the other pieces were removed. I did notice that the query string and fragment are removed from the display but the URL added to the "a" tag retains the values when using the "link" formatter.
Comment #40
circuscowboy commentedI have run into this problem too. I can confirm the following:
The options seem to be to add the query to the URL like with the attached patch or to figure out what entity token is doing with link and get it to do something more.
I have read over the issue that made the change to this area (https://www.drupal.org/project/link/issues/2428185#comment-9710045) and it seems like the purpose of the change was to store the original data and ensure the function always acts on those. I believe my patch still fulfills this but also adds the string to the URL.
Please let me know if there is more I should do in order to complete this issue.
Comment #42
circuscowboy commentedSo seeing the results of the tests show me that this is failing. It makes me think that the fix should probably be upstream - on entity token in my case.
Any thoughts or suggestions.
Comment #43
circuscowboy commentedI have currently solved the issue by moving the url() code into a hook_tokens_alter() to recreate the URL with the query and fragments.
This will be a fine solution for me until D7 EOL.
In my opinion this can be closed (won't fix) as this issue seems to lie in how entity token deals with the link. Perhaps the link module should identify to entity tokens that the result is a link for them to process. As I mentioned above entity token considers the link a text field.
Thanks
Comment #44
chegor commentedWe are using Link 7x-1.7 with patch from comment #9
On our project, it fixed the case when we need a linked image to OpenStreetMap URLs. We use the token [node:field-osm-map-link:url]
Comment #45
emersonreis.devAdding a patch that adds a new URI property so I can use it on the rule I have that redirects the node to the URL on the Link field. I am aware this is far from the final solution to the initial issue but thought I would add this here in case it can help someone else.
Comment #46
codebymikey commentedGenerated a new patch which addresses the issue with the
urlreturning a value without the extra parameters like query string, fragment etc. (e.g. in tokens, or just as part of a regularentity_metadata_wrapper()->url_field-type call)The property now makes use of a dedicated
getter callbackto resolve the correct value similar to theentity_metadata_entity_get_properties()callback.Also updated the display URL for external paths to include the query string.
The #40 patch was applying the query strings twice in specific scenarios especially on the display url for relative paths, so I believe this patch should pass tests now, however dedicated tests might still be needed, but I'm not sure.
Comment #47
emersonreis.devI have seen the same behaviour as mentioned by codebymikey on #46, the patch from #40 duplicates the query strings in specific scenarios.
Comment #48
damienmckennaThe patch still applies, we just need test coverage to make sure it works as expected.
Comment #49
jedihe commentedI found that the testbot is ignoring test classes that depend on 3rd party modules. The testbot console output for #46 shows it clearly: no tests from LinkTokenTest or LinkEntityTokenTest classes.
I just created #3208738: Missing proper test_dependencies, testbot ignoring some tests to resolve that, once that is resolved, we should be in a better position to identify how to properly test for the problem at hand.
Comment #50
jedihe commentedJust scheduled a re-test for #46, now that test_dependencies are properly declared in link.info.
Comment #51
damienmckennaThe tests pass, now we just need test coverage for the changes.
Comment #52
jedihe commentedAdding a test to verify that #46 is a proper fix. Interdiff is just --tests-only.patch
Comment #53
damienmckennaThis is great, thank you!
I think the test coverage could be refactored by extending LinkSanitizeTest instead of adding a whole extra test file for this.
Comment #54
jedihe commentedUpdating the patch with feedback from #53. Interdiff against #46 is the same as the --test-only patch.
I couldn't do all test runs I wanted to for this one, due to a strange ISP issue. Let's see what the testbot says.
Comment #55
jedihe commentedLooking good! let's please review.
Comment #56
damienmckennaSome slight rearranging of the tests.
Comment #57
damienmckennaAdditional test coverage.
Comment #59
damienmckennaExtended the LinkSanitizeTest coverage some more.
Comment #60
damienmckennaComment #62
damienmckennaBack to green! Awesome work, everyone!
Comment #63
Anonymous (not verified) commentedThe issue doesn't seem entirely fixed. We tested version 7.x-1.9, and found an issue when we enter 2 query parameters with the same name (see param2 below):
https://www.test.com?param1=test¶m2=anothertest¶m2=onemoretest
It gets outputted as:
https://www.test.com/?param1=test¶m2=onemoretest%20%20%20¶m1=tes...
We're using "Title, as link".
Comment #65
damienmckenna@tpat2: Please open a new issue and we'll look into this further.
Comment #66
Anonymous (not verified) commentedSorry for the delay, I've created a new issue:
https://www.drupal.org/project/link/issues/3210438
Comment #68
summit commentedHi,
I am sorry to have to report that in my situation the query string is still not shown anymore in latest dev. and version 1.9.
I will open a new issue for this, because this issue is closed.
On version 1.7, this patch worked for me https://www.drupal.org/project/link/issues/1321482, I added this https://www.drupal.org/project/link/issues/1321482#comment-5443624
The new issue is: https://www.drupal.org/project/link/issues/3257043
Thanks for your reply there in advance.
Greetings, Martijn