? Success

User tests: Successful: Unsuccessful:

avatar Disaron
Disaron
18 Nov 2015

Fixing pagination bug according to this

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 Bakual
Bakual - comment - 18 Nov 2015

Setting RTC based on review.

avatar Bakual Bakual - change - 18 Nov 2015
Status Pending Ready to Commit
avatar Bakual Bakual - change - 18 Nov 2015
Milestone Added:
avatar Bakual Bakual - change - 18 Nov 2015
Labels Added: ?
avatar rdeutz
rdeutz - comment - 18 Nov 2015

Not sure if that is right. If you have SEF on then start is used and when off then limitstart. So I think we need to test that, wouldn't merge that on review

avatar Bakual
Bakual - comment - 18 Nov 2015

Current code is wrong for sure. Doesn't make sense to take the "start" value and save it into the $limit variable.
limitstart is a proxy to start as far as I know. The router does rewrite that somewhere if I remember right. If not, we would need an additionally case limitstart which could be added to the start one. But it would be a different issue unrelated to this PR.

avatar rdeutz
rdeutz - comment - 19 Nov 2015

But if we don't set $limit it is always 0 so $limitstart is also 0, doesn't makes sense either.

avatar Disaron
Disaron - comment - 19 Nov 2015

But if we don't set $limit it is always 0 so $limitstart is also 0, doesn't makes sense either.

But we set $limit, this fix set $start too. For what reason we set $start to $limit ("warm" vs "soft")?

avatar Disaron
Disaron - comment - 19 Nov 2015

For example: we have list.limit = 20 and list.start = 0. We set in url ?start=20.
Not fixed code must do this:
list.start = 0 (because we don't assign list.start)
list.limit = 20 (because we assign start to $limit)
Finally we have first 20 records instead of second 20.

avatar rdeutz
rdeutz - comment - 19 Nov 2015

@Disaron it is not that there isn't a problem to fix, but around this part of the code looks odd and doesn't makes sense at all

avatar Disaron
Disaron - comment - 19 Nov 2015

@rdeutz however it fixes bug described here
And base code is really really strange and not logical.
IMHO not logical code must be refactored or as minimum commented :smile:

avatar rdeutz rdeutz - change - 19 Nov 2015
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2015-11-19 11:19:50
Closed_By rdeutz
avatar rdeutz rdeutz - close - 19 Nov 2015
avatar joomla-cms-bot joomla-cms-bot - close - 19 Nov 2015
avatar rdeutz rdeutz - reference | 7071e94 - 19 Nov 15
avatar rdeutz rdeutz - merge - 19 Nov 2015
avatar rdeutz rdeutz - close - 19 Nov 2015
avatar joomla-cms-bot joomla-cms-bot - change - 19 Nov 2015
Labels Removed: ?
avatar Disaron Disaron - head_ref_deleted - 19 Nov 2015

Add a Comment

Login with GitHub to post a comment