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.Before #330748: Remove $limit from theme_pager* themes got the number of items displayed on the page when a pager was displayed, so they can generate pagers like "showing 30 of 2345". After that patch landed there is no way to do that. You cannot reverse engineer the number of items on a page from the number of total items and the total number of pages, because you don't know how many are displayed on the last page. When you have 10 items on 2 pages for example, these can be either 9, 8, 7, 6 or 5 items displayed per page.
The pager designed for the d7ux admin pages also uses such a format "Showing 1-50 of 234 | Next 50 >", which has the number of items on the page twice. So as Earl pointed out (and was ignored) on #8 on #330748: Remove $limit from theme_pager*, we actually need a way to access this data. Since all other data is passed on via global variables, here is another one. Welcome $pager_limits. Since the individually passed variable was deemed inappropriate and all other pager variables are handled in globals, this seems like the most logical way to fix what we have there.
| Comment | File | Size | Author |
|---|---|---|---|
| pager-limit.patch | 2.06 KB | Gábor Hojtsy | |
Comments
Comment #1
webchickMarking needs review to let testing bot have a crack at it.
Comment #2
BerdirIt is not yet possible, but we try to change the hook_query_alter hook so that it is called with the Extenders. That means that someone could create a module that would change $limit for some or even all queries. If that is done, the hard-coded $limit value passed to theme() would be wrong and there is no way to figure this out.
Comment #3
Gábor HojtsyBerdir: well, then looks like that feature was removed on a promise of something else which is not there yet. Since the paged queries would go through either of the functions modified by this patch, this would still collect the correct limit values and make it available just like any other pager variable. Using globals is not nice, but all of other pager values use globals, so I am not set out to save the world with this one.
Comment #4
BerdirNow that I'm actually looking at the patch, yes this might work with globals, if you use by reference for that value, something like
Doing by reference for pager_query() should not be necessary, a) it's not possible to change the value there, b) it will go away anyway. However, I'm not sure what will actually happen when we do that by reference thing. I assume the query object will not be removed from memory....
If re-adding the param to theme_pager() would be an option, one could do something like:
However, that is rather fragile, as it would need to be done after execute().
Comment #5
Gábor Hojtsy@Berdir: since you are advocating that modifiable pager feature, can you please try that out. This patch is actively blocking nice pagers for the Drupal admin theme which is already in Drupal 7. Follow up related issue at #538788: Implement Seven pagers as designed. This is a hard dependency for that one.
Comment #6
Gábor HojtsyFrom IRC:
[13:49] GaborHojtsy: Berdir: from #2, you seem to advocate we change how the patch is written in expectation of some other change
[13:49] GaborHojtsy: Berdir: so I don't understand why is the current patch not right, since your other change can always make your required patch here too
[13:50] GaborHojtsy: Berdir: I mean it is a simple enabler patch for the admin theme pager, and its being hold down for compatibility with a non-existent feature
[13:51] Berdir: GaborHojtsy: forget #2
Again, I think this is too simple to sit here for months. Nobody come out with other complaints, it enables a nice pager in Seven, so what are we waiting for? I'll try to ask others to look at this.
Comment #7
BerdirOk. I'm not yet sure how the modifiable pager will exactly work and it might not even be affected by this, let's not postpone it. The changes in this patch are simple, the tests pass, this looks good to go.
The changes to pager_query() might not even be necessary, as there is only a single call to pager_query() remaining in search.module. However, it does not hurt and we can remove that line together with the rest of pager_query() once search.module is dbtngified.
Comment #8
webchickCommitted to HEAD. Thanks.
I'm tagging "Needs documentation" but mostly just to verify that there is nothing in the upgrade docs that mentions having taken this out. If so, just remove it, since this now behaves as it did in D6 and below afaik.
Comment #9
emmajane CreditAttribution: emmajane commentedI have removed the reference from:
http://drupal.org/update/theme/6/7
I don't see any reference to this patch in:
http://drupal.org/node/224333
I'm marking this issue as fixed and removing the issue tag "needs documentation."