Steps to reproduce:
- On a clean Drupal installation, use the "Devel generate" module to create 100 nodes of type "Story".
- Go to admin/content/node and make sure all nodes are promoted to the frontpage.
- Now, if you go to the frontpage, in the bottom pager you'll see 9 (out of 10) pages listed, but no ellipsis at the end.
The issue appears to be in the theme_pager function, at the point where it checks whether to append an ellipsis element at the end of the list or not.
Comment | File | Size | Author |
---|---|---|---|
#55 | pager_ellipsis_D8-470428-55-complete_0.patch | 1.69 KB | Sivaji_Ganesh_Jojodae |
#48 | pager_ellipsis_D8-470428-44-complete.patch | 1.74 KB | koence |
#44 | pager_ellipsis_D8-470428-44-complete.patch | 649 bytes | koence |
| |||
#44 | interdiff-470428-38-44.txt | 649 bytes | koence |
#38 | pager_ellipsis_D8-470428-38-complete.patch | 1.74 KB | koence |
Comments
Comment #1
foutrelis CreditAttribution: foutrelis commentedUnless I'm missing something, we can simply check if $pager_last contains a smaller number than $pager_max.
Comment #2
OpenChimp CreditAttribution: OpenChimp commentedIt seems like the above patch has been committed, but it is not quite correct.
Instead of:
It should read: (note $pager_max+1 in the conditional)
In D6 this will be:
Comment #3
casey CreditAttribution: casey commentedPager in D7 has changed a lot; does the issue still exist? If not you can move this to D6.
Comment #4
szt CreditAttribution: szt commentedThe issue is exists, #2 solves it, i've turned into patch.
Comment #5
Désiré CreditAttribution: Désiré commentedGreat, it seems work.
But, please see the coding stnadards for operators: http://drupal.org/coding-standards#operators
And this issue needs automated tests.
Comment #6
szt CreditAttribution: szt commented...again with using coding standards
Comment #7
parthipanramesh CreditAttribution: parthipanramesh commentedWorks fine. Thanks.
Comment #8
David_Rothstein CreditAttribution: David_Rothstein commentedHaven't tested Drupal 8, but the same code exists there, so I assume it would need to be fixed there first.
Comment #9
cac2s CreditAttribution: cac2s commentedpatch for 7.26
Comment #11
cac2s CreditAttribution: cac2s commentedComment #12
cac2s CreditAttribution: cac2s commentedComment #13
cac2s CreditAttribution: cac2s commentedComment #14
cac2s CreditAttribution: cac2s commentedComment #15
cac2s CreditAttribution: cac2s commentedComment #16
MrHaroldA CreditAttribution: MrHaroldA commentedWorks perfectly!
Comment #18
David_Rothstein CreditAttribution: David_Rothstein commentedStill seeing similar code in Drupal 8, and no indication from anyone that the bug doesn't exist there.
Comment #19
cac2s CreditAttribution: cac2s commentedComment #20
cac2s CreditAttribution: cac2s commentedComment #21
MrHaroldA CreditAttribution: MrHaroldA commentedThe test failed on queries, so re-testing.
Comment #23
cepinos CreditAttribution: cepinos commentedThis patch fix the problem, I tested it on Drupal 7.
Comment #25
alx_benjamin CreditAttribution: alx_benjamin commentedIt looks like the proposed patches are for D7.
So why is this issue in D8 version?
You cannot test D7 patches against D8 and expect them to pass.
Are we not supposed to create a new issue with correct Drupal version and submit
patches with correct versions so they could be tested properly?
Correct me if I'm wrong
-------------------------------------------------
Sponsored by http://reallifedesign.co.uk/
Comment #26
alx_benjamin CreditAttribution: alx_benjamin commentedCannot replicate in D8. So moving this to original D7 version
-------------------------------------------------
Sponsored by http://reallifedesign.co.uk/
Comment #28
szt CreditAttribution: szt commentedI've posted exactly the same patch in #6 and was confirmed in #7.
The issue needs the D8 fix now.
Comment #29
szt CreditAttribution: szt commentedComment #30
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedComment #31
szt CreditAttribution: szt commentedOk, I've looked after: the bug exists in D8, here is the patch for it.
I've tested, works well.
Comment #32
koence CreditAttribution: koence at XIO commentedEllipsis is shown indeed in D8.
Tested with and without patch.
With patch:
Without patch:
Comment #33
Wim LeersComment #34
Wim LeersSorry, I think this needs tests before it can be committed. We want to avoid this from regressing again.
You'll probably want to expand
\Drupal\system\Tests\Pager\PagerTest
.Comment #35
koence CreditAttribution: koence at XIO commentedI'll work on the test
Comment #36
koence CreditAttribution: koence at XIO commentedComment #37
Wim LeersGreat! But we also need a test for the case where the ellipsis is expected to not yet appear.
Comment #38
koence CreditAttribution: koence at XIO commentedTest has been updated!
Comment #39
Wim LeersClever! :) Love it!
Excellent test — hope to see you around more often in the D8 core issue queue :)
Comment #42
alexpottThis needs to be public. Yes it still works with protected and I know that another method in this test is incorrectly marked as protected but we should still get this one correct.
Comment #43
Wim LeersOops, should've caught that. Sorry, Alex.
Comment #44
koence CreditAttribution: koence at XIO commentedPatch has been updated!
Comment #45
koence CreditAttribution: koence at XIO commentedComment #47
koence CreditAttribution: koence at XIO commentedComment #48
koence CreditAttribution: koence at XIO commentedMade a mistake with previous patch in #44.
Should be ok now.
Function changed from protected to public.
Comment #49
pjbaertLooks like this is rtbc.
Function is now public as requested in #42
Comment #50
alexpottManual testing proves this fix is correct. Nice work. Committed 88470f9 and pushed to 8.0.x. Thanks!
Comment #52
MrHaroldA CreditAttribution: MrHaroldA at iO commentedThe patch in #5 and #23 (same patch) are already reviewed and tested a log time ago, including by me ;)
Comment #54
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedThe tests need to be backported from Drupal 8 also.
Comment #55
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedPatch for D7 with test.