Currently when you use query parameters in different views path options sometimes it works, but other times it does not. It appears to work when you use the "Output this field as a link" option, but not with the trimmed "Add read-more link" or the view's "More" link.
Steps to reproduce field trimmed "Add read-more link" issue:
- Create a view
- Add a filed
- In the Rewrite Results check "Trim this field to a maximum length"
- Check "Add a read-more link if output is trimmed."
- Enter a path with a query parameter (ie: `some/page?filter=value`) for the "More link path"
Steps to reproduce views "More" link issue:
- Create a view
- Change the "More link" and check the "Create more link" option
- Change the view "Link display" option to "Custom URL"
- Enter a path with a query parameter ((ie: `some/page?filter=value`) for the "Custom URL"
When you go to the view both of these links will go to `some/page%3Ffilter%3Dvalue` which will return a Page not found error.
Comment | File | Size | Author |
---|---|---|---|
#17 | more_links_with_query-2210663-17.patch | 6.02 KB | freelock |
| |||
#15 | more_links_with_query-2210663-15.patch | 6.02 KB | freelock |
#11 | more_links_with_query-2210663-11.patch | 6.01 KB | freelock |
| |||
#9 | more_links_with_query-2210663-9.patch | 5.91 KB | joelpittet |
| |||
#6 | views-more-links-with-query-params-2210663-6.patch | 6.03 KB | LittleRedHen |
Comments
Comment #1
jojonaloha CreditAttribution: jojonaloha commentedThe attached patch changes the views_handler_field more link logic to re-use the render_as_link() method since the logic should be the same. It also modifies the views_plugin_display::render_more_link() method to parse the url, in the same way render_as_link() does, to extract the url options before passing it to url().
Comment #2
squarecandy CreditAttribution: squarecandy commentedThis was very helpful and worked for me.
Comment #3
Simon Georges CreditAttribution: Simon Georges at Makina Corpus commentedCan we consider it as major (to give it more visibility) since it breaks the custom URL link functionality and it seems there is no workaround for it?
Anyway, the patch worked for me too, and applies properly to the 3.x-dev version, but to the 3.10 (and I suspect the 3.11) version as well. Changing status to "RTBC".
Comment #4
bellesmanieres CreditAttribution: bellesmanieres commentedWorking as expected for me too.
Comment #5
jmev CreditAttribution: jmev commentedThis is not working for me. I have a view block which displays has the "Create more link" option selected, so it creates a link at the bottom of the page. The view has a contextual filter of "Content: Author uid". In the user page, the view block is displayed, and it shows the user's first 5 items. When you click the more link, it gives you a 404 error. In the preview the more link shows "/users/[uid]/posts", but in the page that shows the block, it shows "/users/all/posts". Neither one works, I get the page not found error. I've applied the patch but no luck. I also checked the menu_router table and see the path listed there.. Could you suggest another approach or test I can run?
Comment #6
LittleRedHen CreditAttribution: LittleRedHen commentedI tried this against both the latest 7.x-3.x and the 7.x-3.13 release, in a view with the 'add more link' enabled and a custom link display that includes query parameters derived from contextual filters.
I got a bunch of undefined index error messages related to non-existent link property indices: 'alt', 'target', 'link_class', 'external', which are not set on the custom link display property.
Here's a modified version of the patch from #1 that checks whether those properties have been set before attempting to use them. With this version applied, my custom link display worked correctly with no errors.
Comment #7
tolu_ CreditAttribution: tolu_ commented#6 works great!
Comment #8
joelpittetLooks like some recent commits made this patch fail.
Comment #9
joelpittet3-way merge auto-resolved. Putting it back to RTBC from #3
If this patch could be simplifed I bet it would much easier to get committers to review and commit. Any suggestions? That or automated tests always help bug reports.
Comment #10
alek123 CreditAttribution: alek123 commentedBut It works!. Drupal 7.43, Views 7.x-3.13
Comment #11
freelockHi,
Patch updated to apply to views-7.x-3.18.
Cheers,
John
Comment #12
joelpittetThanks @freelock. Just compared the diffs and spotted we may have made a mistake.
We shouldn't have dropped the
check_plain()
here. Could you add it back?Comment #13
joelpittetNM, That code was moved up in the code base because the
$text
is returned earlier on it's own. Back to RTBC.Comment #14
freelockYup, and in our case, it was the extra check_plain() that was breaking a link we were trying to pass...
Comment #15
freelockPatch updated for Views-7.x-3.20.
Comment #17
freelockWhoops, typo... updated patch.
Comment #18
freelock... and re-trigger the testbot...
Comment #19
Chris Matthews CreditAttribution: Chris Matthews as a volunteer commentedThe patch in #17 to views_handler_field.inc and views_plugin_display.inc still applies and after walking through the initial steps to produce after the patch is applied I no longer get a 404 so taking the liberty to set back to RTBC to hopefully get in the next 7.x-3.21 release.
Comment #20
DamienMcKennaI think test coverage would be useful here, and I think it could be added to ViewsHandlerFieldTest and ViewsPluginDisplayTestCase to match where the bugs are.
Comment #21
DamienMcKennaRemoving this from the queue for the next stable release, only because there isn't test coverage yet and we want to make sure we don't accidentally introduce more regressions.