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:

  1. Create a view
  2. Add a filed
  3. In the Rewrite Results check "Trim this field to a maximum length"
  4. Check "Add a read-more link if output is trimmed."
  5. Enter a path with a query parameter (ie: `some/page?filter=value`) for the "More link path"

Steps to reproduce views "More" link issue:

  1. Create a view
  2. Change the "More link" and check the "Create more link" option
  3. Change the view "Link display" option to "Custom URL"
  4. 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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jojonaloha’s picture

Status: Active » Needs review
FileSize
4.71 KB

The 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().

squarecandy’s picture

This was very helpful and worked for me.

Simon Georges’s picture

Priority: Normal » Major
Status: Needs review » Reviewed & tested by the community

Can 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".

bellesmanieres’s picture

Working as expected for me too.

jmev’s picture

This 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?

LittleRedHen’s picture

I 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.

tolu_’s picture

#6 works great!

joelpittet’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Looks like some recent commits made this patch fail.

patching file handlers/views_handler_field.inc
patching file plugins/views_plugin_display.inc
Hunk #1 FAILED at 2570.
1 out of 1 hunk FAILED -- saving rejects to file plugins/views_plugin_display.inc.rej
joelpittet’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
FileSize
5.91 KB

3-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.

alek123’s picture

>> patch -p1 < more_links_with_query-2210663-9.patch.txt
patching file `handlers/views_handler_field.inc'
patching file `plugins/views_plugin_display.inc'
Hunk #1 succeeded at 2570 with fuzz 1.
Hunk #2 succeeded at 2613 with fuzz 1 (offset -17 lines).

But It works!. Drupal 7.43, Views 7.x-3.13

freelock’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
6.01 KB

Hi,

Patch updated to apply to views-7.x-3.18.

Cheers,
John

joelpittet’s picture

Status: Needs review » Needs work

Thanks @freelock. Just compared the diffs and spotted we may have made a mistake.

-        return theme($theme, array('more_url' => $path, 'new_window' => $this->use_more_open_new_window(), 'link_text' => check_plain($this->use_more_text()), 'view' => $this->view));
+        return theme($theme, array('more_url' => $path, 'new_window' => $this->use_more_open_new_window(), 'link_text' => $text, 'view' => $this->view));
       }

We shouldn't have dropped the check_plain() here. Could you add it back?

joelpittet’s picture

Status: Needs work » Reviewed & tested by the community

NM, That code was moved up in the code base because the $text is returned earlier on it's own. Back to RTBC.

freelock’s picture

Yup, and in our case, it was the extra check_plain() that was breaking a link we were trying to pass...

freelock’s picture

Patch updated for Views-7.x-3.20.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 15: more_links_with_query-2210663-15.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

freelock’s picture

Whoops, typo... updated patch.

freelock’s picture

Status: Needs work » Needs review

... and re-trigger the testbot...

Chris Matthews’s picture

Status: Needs review » Reviewed & tested by the community
Parent issue: » #2960871: Plan for Views 7.x-3.23 release

The 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.

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

I think test coverage would be useful here, and I think it could be added to ViewsHandlerFieldTest and ViewsPluginDisplayTestCase to match where the bugs are.

DamienMcKenna’s picture

Removing 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.