? ? Success

User tests: Successful: Unsuccessful:

avatar Disaron
Disaron
18 Nov 2015

Sometimes pagination in JModelList don`t work. Usually it happens when you are on first page and has no active filters (if it present) in frontend. Reason that list.start state is cleared in populateState method.

Second commit fix pagination in users list com_users when you activate some filters.

avatar Disaron Disaron - open - 18 Nov 2015
avatar Disaron Disaron - change - 18 Nov 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 Nov 2015
Labels Added: ? ?
avatar joomla-cms-bot
joomla-cms-bot - comment - 18 Nov 2015

Please add more information to your issue. Without test instructions and/or any description we will close this issue within 4 weeks. Thanks.
This is an automated message from the J!Tracker Application.

avatar infograf768
infograf768 - comment - 18 Nov 2015

Please first test #8467

avatar Disaron
Disaron - comment - 18 Nov 2015

infograf768, tested. No this is not helps in frontend (solved by 3aee886). It helps only for com_users users list, yes. But rows removed in 7bd23ff solves this too, and this rows are unnesesary because JModelList::populateState do it.

avatar Bakual
Bakual - comment - 18 Nov 2015

Can you provide exact steps to reproduce the issue you see?

The first commit almost certainly is wrong. The value for start should be cleaned for sure, removing that is no option as it would be a security issue allowing uncleaned input into the states.

The second commit looks wrong as well. The PR #8467 should indeed have fixed that.

avatar Disaron
Disaron - comment - 18 Nov 2015

First commit break my mind at two weeks after migration to 3.4.5. I think for this reasons populateState in com_content (articles list) in frontend totally overriden (has no parent::populateState).

Steps to reproduce:
Create custom model list with filtering fields in frontend, but don`t select any filters (limit also).
Fill some items to see pagination.
Select 1st page.
Drop session (logout or anything else, I have HTTP-SSO and automatically log in after logout but with new session), but not go from this page.
Login. (still on this page)
Try to go to 2nd page.
1st page still selected.

Well I see another solution (no time to create pull request now):

case 'start':
    $value = $inputFilter->clean($value, 'int');
    break;

For second commit I think that this commit and this lines says you why it possible ;) And yes, #8467 fix it too.

avatar mbabker
mbabker - comment - 18 Nov 2015

com_content had overridden the parent populateState method long before 3.4.5's release, the two topics are unrelated.

The line you've removed in JModelList actually is 100% irrelevant to processing. Even if you inject a value for the list.start model state via the request, it is overwritten by the limitstart query variable and its default value if it isn't sent. Even so, if it were the actual value used then list.start must be an integer given its use cases and that is exactly what is happening there.

avatar Disaron
Disaron - comment - 18 Nov 2015

Sorry but happening what? What do with start - nothing. $limit is assigned, $start goes to blackhole.
Current code in staging:

case 'limit':
case 'start':
    $limit = $inputFilter->clean($value, 'int');
    break;

$limit = - seriously? Maybe must be $value =? ;)
No it`s not, because
By now need to add separate option

case 'start':
    $value = $inputFilter->clean($value, 'int');
    break;

I understand that my first fix is incorrect, I do not insist to merge this PR because it's fix really wrong. But bug is present.

avatar Disaron
Disaron - comment - 18 Nov 2015

com_content had overridden the parent populateState method long before 3.4.5's release, the two topics are unrelated.

Then you must know, that in current release pagination in com_content goes to first page when you reopen list? Because list.start not save in states (maybe it's feature, I don't know...)

avatar Bakual
Bakual - comment - 18 Nov 2015

True, that part with the start and limit indeed looks wrong. Can you do a PR to fix that alone, without anything else and without removing filtering for the start.

avatar Bakual
Bakual - comment - 18 Nov 2015

Then you must know, that in current release pagination in com_content goes to first page when you reopen list? Because list.start not save in states (maybe it's feature, I don't know...)

That is indeed by design. The pagination isn't saved in the userstates in frontend views.

avatar Disaron
Disaron - comment - 18 Nov 2015

Done: #8481

avatar Bakual
Bakual - comment - 18 Nov 2015

Closing this one as we have a PR that deals only with the issue at hand

avatar Bakual Bakual - change - 18 Nov 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-11-18 19:16:21
Closed_By Bakual
avatar Bakual Bakual - close - 18 Nov 2015

Add a Comment

Login with GitHub to post a comment