The node link views handlers are currently using the same function for creating links as all other fields do when they use the make_link option.
This usually works fine, but if you have a replacement pattern that for some reason interferes with the link in any way, the links can break.
In my case we had a view with two terms as arguments and we added the node edit link. The node edit link has a destination GET parameter which links back to the view, and since that parameter gets url encoded, we got things like %2 in the destination variable. That got replaced with the term name in the link, which caused some problems =)
The attached patch overrides the render_as_link function in the node_link field handler and empties the tokens array. That way, no tokens can interfere with the link output. The change should be quite unintrusive as far as other functionality goes since it only happens in the node link handler.
Comment | File | Size | Author |
---|---|---|---|
#60 | views-rendered_links_token-1009646-60.patch | 5.14 KB | nagy.balint |
#45 | views-rendered_links_token-1009646-44-with-tests.patch | 3.83 KB | geertvd |
#45 | views-rendered_links_token-1009646-40-with-tests.patch | 3.65 KB | geertvd |
#44 | views-rendered_links_token-1009646-44.patch | 1.29 KB | ShaunDychko |
#40 | interdiff_38-40.txt | 670 bytes | heddn |
Comments
Comment #1
fabsor CreditAttribution: fabsor commentedJust setting status.
Comment #2
merlinofchaos CreditAttribution: merlinofchaos commentedDoes this break if you turn on link rewriting and actually put a token in the link?
Comment #3
bohz CreditAttribution: bohz commentedSubscribing.
I seem to have a similar problem with an edit link with destination portion of the path like:
/edit?destination=taxonomyFtermF785
while it should read something like:
/edit?destination=taxonomy/term/785
I have applied the patch above to
views_handler_field_node_link_edit.inc
and now it seems to work as expected.
Thanks a lot
Comment #4
ordually CreditAttribution: ordually commentedSubscribing. I had this problem and applying the patch works for me.
I tried to investigate Merlin's question on tokens and find that the "Output this field as a link" options "Replacement patterns" listing shows arguments and fields in the view, but I don't see tokens listed. I tried tossing in a "[nid]" token for yuks and it didn't replace pre or post-patch.
@merlin: Thanks for the great module and incredible support!
Comment #5
merlinofchaos CreditAttribution: merlinofchaos commentedI'm considered that indiscriminately removing the tokens is a bad idea.
I think we should check to see if the checkbox is there, meaning the link was prepopulated. We certainly wouldn't want tokens in that case anyway. It doesn't burden the UI, and tokens will still work as expected if the link is rewritten normally.
Comment #6
braindrift CreditAttribution: braindrift commentedSubscribing
Comment #7
braindrift CreditAttribution: braindrift commented@merlinofchaos
What sense has the option "Output this field as a link" with its token replacement for a field, which will be rendered as link anyway with its own path etc.?
IMHO this patch does exactly what it should do. Maybe it should also remove the option "Output this field as a link".
Comment #8
merlinofchaos CreditAttribution: merlinofchaos commentedBecause if I have chosen to rewrite the link, even if a default link was provided for me, I probably want tokens. I can't rewrite a link as node/[nid] without them.
Comment #9
braindrift CreditAttribution: braindrift commentedYou can not rewrite a node-link. This option has no effect for this fields.
Comment #10
braindrift CreditAttribution: braindrift commentedOK, I was wrong. This option has only an effect, if the user has no permission to edit/delete the node. In this case the rewritten path is used.
Comment #11
braindrift CreditAttribution: braindrift commentedIs there a special reason for the argument tokens not to be placed inside square brackets?
Comment #12
braindrift CreditAttribution: braindrift commented@merlinofchaos: What about to put the tokens for arguments also inside square brackets (e.g. %1 => [%1])?
Comment #13
merlinofchaos CreditAttribution: merlinofchaos commentedChanging that would break the thousands or hundreds of thousands of sites that assume the existing behavior.
Comment #14
braindrift CreditAttribution: braindrift commentedThanks for your reply.
What about an auto-repair function? This function could search for the old tokens and automaticly raplace them by the new one.
The fact is that encoded URLs with something like %2F must not be replaced.
Comment #15
ayalon CreditAttribution: ayalon commentedI have the same problem! I have some Fs in the edit links
The edit link looks like this:
http://www.test.com/node/28/edit?destination=nodeF21
Originally
http://www.test.com/node/28/edit?destination=node%2F21
%2 gets replaced.
If I understand merlinofchaos correctly I shouldn't apply the patch. But is there a solution to display the URLs correctly?
Comment #16
rnj786 CreditAttribution: rnj786 commentedI have a similar problem with node edit link, when using the view with exposed filter and AJAX turned ON the node edit link becomes too long and the page says Request-URI Too Large.
How can i get rid of the Destination part ?
Comment #17
rnj786 CreditAttribution: rnj786 commentedI got the query string removed from the URL, that solved the problem for now but i don't know if that would cause any parallel issues.
Removed line 26
// $this->options['alter']['query'] = drupal_get_destination(); from the file
views_handler_field_node_link_edit.inc
Please point out if there is a better solution than removing the destination query string.
Comment #18
tbenice CreditAttribution: tbenice commentedhi rnj786,
it's a bit off topic here but to remove the destination you could rewrite the edit link output and set it to output a link 'node/[nid]/edit'. you'd of course have to pull in the nid field and set it to exclude display.
Comment #19
igor.ro CreditAttribution: igor.ro commentedPatch work perfect but I found out the same bug with node delete link
so provide updated patch
Comment #20
igor.ro CreditAttribution: igor.ro commentedComment #21
braindrift CreditAttribution: braindrift commentedThis should not be the solution for the problem, because this would also prevent the replacement of tokens that must be replaced.
e.g in the "Link class" or the "Alt text".
IMHO the better solution would be to change the way the token replacement is performed in the views_handler_field.inc on line 1154.
old code:
new code:
Comment #22
kla2t CreditAttribution: kla2t commentedsubscribe
Comment #23
khaldoon_masud CreditAttribution: khaldoon_masud commentedThe correct patch to work for "node edit" and "node delete" link:
Comment #24
jrao CreditAttribution: jrao commentedHere's a patch for 7.x based on #21
Comment #25
FreekVR CreditAttribution: FreekVR commentedI think the best way to fix this for D6 is to decode the URL before processing tokens. This effectively means that default destination paths for node edit links for example will not get URL encoded characters (eg %2F) replaced, and tokens will still work, since %2 is not a valid URL encoding.
The attached patch is rolled against views 6.3.x
Comment #27
Gribnif CreditAttribution: Gribnif commentedThe patch in #24 also corrects the problem in #2004960: rewrite destination parameter when using node delete links.. I think it's RTBC for 7.x.
Comment #28
heddnThis should do the trick. Re-encoding another time causes issues.
Comment #29
heddnAnd picking up something noted in #2220681: render_as_link in views_handler_field applies tokens incorrectly when query is altered. This does not need to be ported to D8 because of https://www.drupal.org/node/2566251.
Comment #31
heddnComment #32
azinck CreditAttribution: azinck commentedSeems to me that #31 won't work. The token replacement *must* be done on the individual values in the query array before it's rendered as an encoded query string. Consider what would happen in #31 if there's a parameter value that contains an ampersand? That ampersand would be decoded, then split on in drupal_get_query_array().
Comment #33
heddnSo far, so good. I edit the node and save it. The URL now looks like: node-test/5?title=notice%20&news=
The ampersand lost its encoding and '&n' isn't a valid character so the contents of the title filter now is 'notice '. Everything from the & and after is lost.
OK, I got my notes written. Now to study things and figure out what is going on and if we can salvage everything after the ampersand. I think, in general, the patch here improves things measurably. But we might be able to get better. The only thing not functional is the loss some of the contents of the exposed filter.
Comment #35
heddnScreen capture of test view:
Comment #36
heddnComment #37
heddnComment #38
heddnComment #39
azinck CreditAttribution: azinck commentedIs there a reason we need to send this through this round trip? Why not just set $options['query'] = $alter['query']?
Comment #40
heddnre #39: I don't recall if the reason is simply because that is how it was done before or if there was a larger reason. Let's see how the tests fair. There seems to be decent test coverage of this part of the field handler.
Comment #41
azinck CreditAttribution: azinck commented#40 looks good and is working for me.
Comment #42
NWOM CreditAttribution: NWOM commented#40 worked for me as well. Thank you for everyone's work on this!
Comment #43
cha0s CreditAttribution: cha0s commentedJust wanted to drop another affirmative for #40 :)
Comment #44
ShaunDychko CreditAttribution: ShaunDychko for Bellin commented#40 didn't quite work for the use case where a destination query parameter in
$alter['query']
already contains a URL encoded entity.og_handler_field_og_membership_link_edit::render_link
from Organic Groups does this by usingdrupal_get_destination()
here:When using an exposed filter with a text field containing a space, the URL will be like
group/node/1237/admin/people?uid=The+Name&state=All
butdrupal_get_destination()
returnsgroup/node/1237/admin/people?uid=The%20Name&state=1
. Notice the plus replaced by %20.The following patch returns to the idea of #31 by decoding the
$alter['query']
array, but makes the decoding recursive since the query parameter can possibly contain nested arrays (seedrupal_http_build_query()
which also accounts for nested arrays).The critique against #31 in #32 was "Consider what would happen in #31 if there's a parameter value that contains an ampersand?" While the url decoding doesn't modify any literal '&' characters, it would change ;amp into '&', which will cause problems since
drupal_get_query_array()
will make erroneous splits on the new literal '&' characters introduced by decoding.Sorry I don't have the answer about what to do here. #40 fails if
$alter['query']
has something that's already url encoded, whereas the patch here fails if a query parameter contains an ampersand. Setting to "needs work" :PComment #45
geertvd CreditAttribution: geertvd at Geert van Dort commentedAdded some test coverage for this issue and applied it to both the patch in #40 and #44.
All of these tests should fail for now.
Comment #49
geertvd CreditAttribution: geertvd at Geert van Dort commentedThis should be $name_field instead of $field_destination
Comment #50
geertvd CreditAttribution: geertvd at Geert van Dort commentedTest only patch where I fixed that typo.
Comment #52
ShaunDychko CreditAttribution: ShaunDychko for Bellin commentedThanks for the tests @geertvd! Here's another go. It works for any url encoded characters! The only limitation I can see is that it won't replace token that are in the keys of the
$alter['query']
array. Would there ever be tokens in the keys? I didn't think so, but if that's important, then maybe this patch could be extended for that requirement. This patch includes the tests from #50.The approach for this patch is to use regex to apply token replacement only on strings not containing a url encoded character, as matched by
/(%[0-9a-fA-F]{2})/
.If a value contains a urlecoded string, that string is decoded since this array is later passed to l(), the documentation for which says that query parameters should not be encoded. From https://api.drupal.org/api/drupal/includes%21common.inc/function/url/7.x
.
Passing url encoded characters to l() results in the percent sign being encoded as %25. For example "John%20Smith" becomes "John%2520Smith".
Comment #54
ShaunDychko CreditAttribution: ShaunDychko for Bellin commentedAdjusted the test to pass a decoded query parameter to l().
Comment #55
jomarocas CreditAttribution: jomarocas commentedI try the patch #54 and try and dont working, i have a views with simple url to edit, with certainly filters, apply and execute the filters, later go to edit, but nothing no works
Comment #56
jomarocas CreditAttribution: jomarocas commentedi Consider this is major and need work, node edit link no working while your apply filters using or not ajax
Comment #57
Richard15 CreditAttribution: Richard15 commented+1
Comment #58
Chris Matthews CreditAttribution: Chris Matthews as a volunteer commentedThe two year old patch in #54 to views_handler_field.inc and views_handler_field_url.test does not apply to the latest views 7.x-3.x-dev.
Comment #59
Chris Matthews CreditAttribution: Chris Matthews as a volunteer commentedThe two year old patch in #54 to views_handler_field.inc and views_handler_field_url.test does not apply to the latest views 7.x-3.x-dev.
Comment #60
nagy.balint CreditAttribution: nagy.balint commentedRerolled.
Comment #61
nagy.balint CreditAttribution: nagy.balint commented