Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Running latest version of 8.x-dev;
- enabled the View which overrides /taxonomy/term/%
- updated pager settings and entered 0 for "Items per page"
- got Warning: Division by zero in pager_default_initialize() (line 123 of core/includes/pager.inc).
on term page, zero nodes displayed.
Comment | File | Size | Author |
---|---|---|---|
#10 | vdc-1958470-10.patch | 5.64 KB | olli |
#10 | interdiff.txt | 803 bytes | olli |
#4 | vdc-1958470-4.patch | 5.63 KB | olli |
#3 | vdc-1958470-3-fail.patch | 1.88 KB | olli |
#3 | vdc-1958470-3.patch | 4.53 KB | olli |
Comments
Comment #1
olli CreditAttribution: olli commentedI saw this too: Install drupal, create an article, change front page pager to "Mini" and set "Items per page" to 0. Visit front page, see the warning and "No front page content has been created yet." message.
Comment #3
olli CreditAttribution: olli commentedI guess it's not so common to use the mini pager when you want to show all items, but it should be possible to expose the "- All -" items per page option.
Comment #4
olli CreditAttribution: olli commentedHere is an alternative that removes pager_default_initialize() and uses updatePageInfo() which is called also when view result is cached.
Comment #6
dawehnerThe fix and the new test looks fine. Did someone tried that with the full pager and maybe even on Drupal 7 views?
Comment #7
alexpott@dawehner can you make it clear if you are rtbc-ing #3 or #4?
Comment #8
olli CreditAttribution: olli commented#4: vdc-1958470-4.patch queued for re-testing.
Comment #9
dawehnerI like that updatePageInfo() is moved away from executeCountQuery!
I'm not sure about that line, as 1 is just the wrong number. We have no clue what the total amount of items is of a mini pager, so there should be no lie about it?
Well, actually the pager calculated the total items, it was not returned from the query. Let's improve the assertion message a bit.
Comment #10
olli CreditAttribution: olli commentedThanks! Here is an improved assertion message. I'd be equally fine removing the line, since I'm not sure if that assertion is really relevant anymore after this patch, but 1 is the number to expect.
Comment #11
dawehnerAre you really sure that the number 1 is expected. The number is somehow random, because like written before we have no clue what the actual value should be. It seems to be wrong just from the DX side.
Comment #12
olli CreditAttribution: olli commentedI'm pretty sure it is 1, there is (just few lines above the assertion):
If we want to keep $view->total_rows NULL, we should revert this change. Should I work on that or do you have better suggestions?
Comment #13
dawehnerI always thought that the total rows is the amount of items which are available, but maybe this is not the case if I read the documentation on ViewExecutable, which also simply could be just broken :) URGS
Comment #14
dawehnerWell, let's keep this then.
Comment #15
catchCommitted/pushed to 8.x, thanks!