This is the same bug that I reported for the 7.x-3.x version of Views module.
On a view with AJAX enabled, and an exposed filter that has default values, if the user chooses to leave the filter's values blank, the view respects user's choise only on the first page of the results, whereas if the user moves to next pages, the result comes back different. This sometimes causes empty pages without pager.
nid title
1 My test article
2 A beautiful day
3 My article is super
4 Master of puppets
5 Fear of the dark
6 This article will fail
7 Awesome article about drupalYou will see that:
But if you click on the 2nd page on the pager, or the 'next' button', you get an empty view: no results and no pager!
Also you can notice that now the exposed filter 'nid' contains the default values 5 and 10.
To make it more clear, make a small change: Edit the view, and change the pager to 1 item per page.
Visit the view page.
You have 3 results, one on each of the 3 pages.
Now type again 'article' into the title filter, and delete both values on nid. Click Apply.
You get a result of 4 nodes, one on each of the 4 pages.
If you visit page 2 (or 'next'), now you don't get an empty page, because there actually exists a 2nd page, but as you can see, the number of pages is reduced from 4 to 2.
I propose a patch that solves the issue.
Please check attached yml file
| Comment | File | Size | Author |
|---|---|---|---|
| #36 | 2648460-36.patch | 550 bytes | tohesi |
| #33 | 2648460-33.patch | 1.07 KB | _utsavsharma |
| #33 | interdiff_31-33.txt | 529 bytes | _utsavsharma |
| #31 | 2648460-view-args-parser_31.patch | 1.22 KB | opi |
| #26 | 2648460-view-args-parser_26.patch | 496 bytes | cslevy |
Comments
Comment #2
efpapado commentedComment #3
dawehner@tim.plunkett, @effulgentsia @alexpott @xjm @cottser and @dawehner discussed that and we think that is "just" normal. I'll review it tomorrow,no worries and we fix it.
We just considered that not important enough to be considered as major compared to other major issues.
Comment #4
efpapado commentedNo disagreements for the priority change :)
As I get it, this line implements a change on the logic. When the GET request is done without ajax, all the parameters get passed on the url, even the non-valued ones. But when the AJAX mechanism ajaxifies the callback, it only passes the valued parameters, discarding the non-valued ones.
I believe this is an inconsistency that should be fixed, and it gets fixed with the patch.
Also please note that the same bug exists on D7 version of Views, I have also opened an issue and proposed a patch there #2646332: AJAX view with exposed filters & default values does not respect user's empty choice (not sure if the maintainers are the same)
Comment #5
efpapado commentedHave you thought about this?
Comment #6
dawehner@efpapado
I agree that we simply should add all query parameters, no matter whether there are values or not. We cannot make any assumptions about other query parameters in the system.
Comment #7
esolitosSince
pair[1]is actually used, shouldn't be there another check to avoid calling the.replace()method onundefined?Comment #8
droplet commentedLooking at patch changes, it may more worth to check out following issue
Comment #9
efpapado commented@esolitos:
No, it is not needed. The array
pairis generated bysplit:pair = pairs[i].split('=');So
pair[1]is never undefined, it just could be empty string.Comment #10
efpapado commentedSo I guess RTBC should come back ;)
Comment #11
droplet commentedPatch #0 isn't safe because the pairs[i] can be `undefined`. Therefore both pair[0] & pair[1] are `undefined`
Comment #12
dawehner@droplet
Well, this is a bug unrelated with this issue, isn't it?
Comment #13
droplet commented@dawehner,
Yes and No. Because it checked pair[1] before that imply assumed to check pair[0] (in pairs[i].split). If pair[1] is exist, pair[0] must has value also.
Comment #14
efpapado commented@daweher
I think droplet is right.
If
queryequals toa=b&then
var pairs = query.split('&')will be an array:pairs[0] = 'a=b'and
pairs[1] = ''// empty stringSo on the iteration, for
i=1pair[0] = ''and
pair[1] = undefinedThis means that
if (pair[0] != 'q')will be valid, but thenpair[1].replace(/\+/g, ' ')will throw error.His patch is more safe.
Comment #16
lendudeWe now have javascript testing available, so this will need tests.
Comment #17
lendudeComment #25
cslevy commentedRe-rolled patch for Drupal 9
Comment #26
cslevy commentedRemoved empty line changes from the end.
Comment #27
Coops_Tested #26 on Drupal 8.9.13 and it works well, thanks.
Comment #31
opireroll for 9.5.0, and added base.es6.js too ; I don't know how to write test though, sorry
Comment #32
_utsavsharma commentedComment #33
_utsavsharma commentedAs of #31, the patch was made for 9.5.x.
Tested and fixed it.
Please review.
Comment #36
tohesi commentedReroll patch from #33 for 10.1.x.
Comment #38
cslevy commentedRe-rolled patch for 10.3.x
Comment #39
cslevy commentedThis is already fixed on 10.3.x. Sorry for the previous patch. it's a mistake.
Comment #40
vinmayiswamy commentedHi everyone,
I’ve followed the steps outlined in the issue description to reproduce the issue. Here’s a summary of my findings:
Commit Analysis:
- The recent commit d251a98 includes a relevant fix in the
views/js/base.jsfile that addresses a similar issue.- The updated code now includes logic to handle cases where query parameters might be empty, which aligns with the changes proposed in the patch for this issue.
Code Comparison:
Original Patch Diff:
Recent Commit Diff:
Findings:
- The recent commit ensures that query parameters are handled correctly even if their values are empty, resolving the issue of incorrect pagination and filtering behavior.
- I have tested the functionality in Drupal 11.x, and the filtering and pagination work as expected.
Given that the recent commit appears to resolve the issue and the functionality works correctly in Drupal 11.x, the patch proposed in this issue may no longer be necessary.
Request for Feedback:
- If there are any additional steps or scenarios that I may have overlooked while reproducing the issue, I would be grateful if you could share them with me.
- I would greatly appreciate any guidance on whether further action is required or if there are any other aspects I should consider.
Thanks!
Comment #41
johnvComment #46
xjmCrediting as per triage in #3.
Nice research @vinmayiswamy! Sorry for the very long delay in response. It does indeed look like #3100826: AJAX pager doesn't work with exposed filter which has a default value may have been a duplicate of this issue.
The final step I would take would be to manually test this on Drupal 10.0 and confirm that the bug can be reproduced, and then again Drupal 10.1 (or even a more supported version like 10.5) and confirm the bug is no longer there.
Once that is done, if it shows this issue fixed, we can close this issue as a duplicate.
Comment #47
smustgrave commentedBelieve this has been resolved.