Problem/Motivation

I think the approach that the views AJAX system takes to determine views arguments is incorrect. Right now when doing AJAX pagination, the view attempts to get all of the contextual view information from the pager link. If ?page=1 is set on the link, it will use that in it's ajax request. If /VIEW-PATH/arg1/arg2/arg3 is set, it will use those three arguments when executing the view.

This makes sense for query strings because they are never aliased and are required to store the pagination state. The problem with arguments is that all of the argument plugins that load context are aware of the underlying route. For example, when a views block is attached to /user/1 (aliased to /member/admin), the plugins can successfully determine that the view needs to be filtered by user id 1.

When an AJAX pagination link is clicked, the arguments are attempted to be determined from the HREF of the link. This is done on the client and has no knowledge of being able to pull an argument value from a route. The result is /member/admin is parsed and "admin" is determined as the argument for the view. Once this filters through to executing the view, the context of this being an entity ID that can be filtered with is lost and the view returns 0 results.

I don't believe we can successfully parse a views arguments on the client without the routing context.

Proposed resolution

When paginating, don't recalculate the views arguments based on the link HREF. Use the same arguments that were present when the view loads.

I believe this makes sense because it's unlikely you would ever want arguments to change between pages.

Remaining tasks

Validate/patch/test.

User interface changes

TBD.

API changes

TBD.

Data model changes

TBD.

CommentFileSizeAuthor
#55 2703771-55.patch609 bytessahil.goyal
#50 interdiff-2703771-49_50.txt631 bytesGauravvvv
#50 2703771-50.patch1.2 KBGauravvvv
#49 drupal-n2703771-49.patch1.23 KBDamienMcKenna
#48 drupal-n2703771-48.patch1.23 KBDamienMcKenna
#47 2703771-46-9-1-x.patch797 bytesChristianSanders
#45 2703771-45-8-9-x.patch696 bytesEricmaster
#44 2703771-44.patch774 bytesEricmaster
#34 2703771-34.patch773 byteschiranjeeb2410
#32 2703771-32.patch1.24 KBSam152
#29 2703771-29.patch1.23 KBSam152
#18 2703771-dont-calculate-views-args-client-side-18.patch547 bytesSam152
#6 2703771-dont-calculate-views-args-client-side-2-core-relative.patch1.49 KBSam152
#2 2703771-dont-calculate-views-args-client-side-2.patch1.52 KBSam152
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sam152 created an issue. See original summary.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Status: Active » Needs review
FileSize
2703771-dont-calculate-views-args-client-side-2.patch1.52 KB

This fix is working for my project.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Title: Drupal.Views.parseViewArgs is not aware of routes whereas argument plugins are. » Views argument incorrect when paginating with a path alias.
Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Title: Views argument incorrect when paginating with a path alias. » Views argument set incorrectly when using AJAX pagination and a path alias
Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Issue summary: View changes
Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
FileSize
2703771-dont-calculate-views-args-client-side-2-core-relative.patch1.49 KB

Relative to core (for my own testing). Please ignore.

Status: Needs review » Needs work

The last submitted patch, 6: 2703771-dont-calculate-views-args-client-side-2-core-relative.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran at PreviousNext for University of Technology, Sydney commented
Status: Needs work » Needs review

Have you tested this with expose filters?

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

As far as I can see, the patch only touches pager links. Haven't manually tested with exposed filters.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Issue tags: +Needs reroll
Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Issue tags: -Needs reroll

#2 applies, #6 was for my use relative to core so I could apply the patch with composer.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Issue tags: +Needs tests

Let's ensure to add tests for that. I would also avoid removing that API to be honest, you know, maybe someone is actually using it.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

It looks like it only used in the context of pagination, no generic kind of view-ajax-link-api, so I'm not sure how changing a views argument would be applicable to that at all, ever.

This is likely to be a huge test with perhaps limited benefit? We're essentially removing a feature and want to asset that it's gone?

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

By the way I looked up the history of the code and it looks like it's left over untouched from the 7.x days. I think it's just an oversight that can be corrected by deleting the "feature".

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

I'm actually more talking about the bug you described here:

I think the approach that the views AJAX system takes to determine views arguments is incorrect. Right now when doing AJAX pagination, the view attempts to get all of the contextual view information from the pager link. If ?page=1 is set on the link, it will use that in it's ajax request. If /VIEW-PATH/arg1/arg2/arg3 is set, it will use those three arguments when executing the view.

Sure I'll RTBC this patch and then a committer will push it back. Believe me, just keep the function around will just make all our life easier :)

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

You mean leave the function there but don't use it anywhere in Drupal?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

Yeah ...

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
FileSize
2703771-dont-calculate-views-args-client-side-18.patch547 bytes

Okay, that makes sense. Thanks for the clarification.

Saphyel’s picture

Saphyel CreditAttribution: Saphyel as a volunteer commented

I tried on my local without your patch and AJAX pagination works fine on 8.2.x. Could you provide details for replicate this ?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

@Saphyel Did you also tried path aliases?

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

Yep, you need a view on a path which is aliased, eg user/1 to member/username, then select one of the contextual filter plugins which grabs an entity ID from the matched route.

Saphyel’s picture

Saphyel CreditAttribution: Saphyel as a volunteer commented

yep you are right guys, also doesn't work the patch I noticed that for some reason in the 2nd page load changes the "view_path" to "/views/ajax", and lose another elements like "page:1" etc...

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jesse.voogt’s picture

jesse.voogt CreditAttribution: jesse.voogt at Plethora commented

I'm experiencing the same symptoms (based on inspecting the Net panel when clicking "show more"), but for Drupal 7. Any chance of making a version of the patch for D7 as well?

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

7.x-3.x version in the views queue. #2805539: Views argument set incorrectly when using AJAX pagination and a path alias

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

slucero’s picture

slucero
he/him
Primary language English
CreditAttribution: slucero commented

Based on the issues mentioned here with view_path being changed, I expect this issue may be solving the same problem from a different approach as #2866386: Assert the view path is set correctly after second ajax request.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
FileSize
2703771-29.patch1.23 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch, 29: 2703771-29.patch, failed testing. View results

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented
Status: Needs work » Needs review
FileSize
2703771-32.patch1.24 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch, 32: 2703771-32.patch, failed testing. View results

chiranjeeb2410’s picture

chiranjeeb2410 CreditAttribution: chiranjeeb2410 at Google Summer of Code commented
Status: Needs work » Needs review
FileSize
2703771-34.patch773 bytes

Rerolled against latest core from #18

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

SocialNicheGuru’s picture

SocialNicheGuru CreditAttribution: SocialNicheGuru commented
Status: Needs review » Needs work

Now I am getting a syntax error in Firefox and Chrome

Chrome:
Uncaught SyntaxError: Unexpected token .

Firefox:
SyntaxError: missing : after property id[Learn More]
Firefox gives an explanation:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors...

When creating objects with the object initializer syntax, a colon (:) separates keys and values for the object's properties.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mbovan’s picture

mbovan CreditAttribution: mbovan at MD Systems GmbH commented
Status: Needs work » Needs review

#22, #27 view_path was fixed in #2820347: Exposed filter reset redirects user to 404 page on AJAX view when placed as a block.

@Sam152, do we still need #34 patch for path aliases or can we close this issue as a duplicate?

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

Both path aliases and pager changed in 8.8 core

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

loopy1492’s picture

loopy1492 CreditAttribution: loopy1492 commented

Worse, the code in #34 seems to be incompatible with some versions of jquery. I'm getting

Uncaught SyntaxError: Unexpected token '.'

on the added line and my text editor is upset about the new nesting.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Ericmaster’s picture

Ericmaster CreditAttribution: Ericmaster as a volunteer commented
FileSize
2703771-44.patch774 bytes

Haven't tested yet but the following patch should fix the Unexpected token issue.

Ericmaster’s picture

Ericmaster CreditAttribution: Ericmaster as a volunteer commented
FileSize
2703771-45-8-9-x.patch696 bytes

The following patch worked for me against 8.9.x

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ChristianSanders’s picture

ChristianSanders CreditAttribution: ChristianSanders at ComputerMinds commented
FileSize
2703771-46-9-1-x.patch797 bytes

Amended a working patch for use on 9.1.x.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
drupal-n2703771-48.patch1.23 KB

Rerolled. Note that the JS has changed notably since the original patch was written, I recommend reviewing the logic to confirm whether it follows the latest JS standards or could be improved.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
drupal-n2703771-49.patch1.23 KB

Rerolled.

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at OpenSense Labs for Drupal India Association commented
FileSize
2703771-50.patch1.2 KB
interdiff-2703771-49_50.txt631 bytes

Fixed CS errors, Attached interdiff for same. Please review.

Status: Needs review » Needs work

The last submitted patch, 50: 2703771-50.patch, failed testing. View results

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

sahil.goyal’s picture

sahil.goyal CreditAttribution: sahil.goyal as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
FileSize
2703771-55.patch609 bytes

reroll patch for 10.1.x

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.