Skip to content

Regression: We can't remove the URL-Parameter "limitstart"#4488

Closed
Bakual wants to merge 1 commit intojoomla:stagingfrom
Bakual:RevertLimitstartRemoval
Closed

Regression: We can't remove the URL-Parameter "limitstart"#4488
Bakual wants to merge 1 commit intojoomla:stagingfrom
Bakual:RevertLimitstartRemoval

Conversation

@Bakual
Copy link
Copy Markdown
Contributor

@Bakual Bakual commented Oct 8, 2014

Issue

In some extensions the user can't get back to the first page once he navigated to a later page.
To my knowledge this doesn't happen in core extensions but other extensions have this reported.
It only happens when SEF URLs are enabled.

Explanation

If an extension uses JModelList::populateState(), the start of the pagination (limitstart) is stored in the session (userstate). That means the value is only changed if the limitstart parameter is present in the request. Due to a recent change (#3725) the limitstart was removed from the request when it was equal to zero. While this works for some extensions that don't use the userstates (like the core extensions), it broke pagination for the extensions which use those session states.

Solution

This PR just reverts the faulty PR and restores the limitstart parameter.

Testing

As this doesn't happen with core extensions, you need to use a 3rd party extension which is affected. You can use my own "SermonSpeaker" to do that. Either install it using the webinstaller or get it from http://www.sermonspeaker.net/download/sermonspeaker-component/sermonspeaker-component-5-2-3.html.
Create multiple sermons (batch copy the existing example sermon) and a menu item and test the pagination.
Make sure you have SEF URLs turned on as non-SEF URLs are not affected.

@garkell
Copy link
Copy Markdown

garkell commented Oct 9, 2014

Tested on a fresh install of 3.3.6 with limit set to 5. 10 articles - tested for features listing and category listings with pagination and all worked fine. Installed my component with parent::populateState in the model and pagination worked fine as well. Thanks again. Cheers.

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4488.

@betweenbrain
Copy link
Copy Markdown
Contributor

@test

Was able to reproduce the issue with @Bakual's extension and the patch does resolve the issue of not being able to navigate to the first page.

@810
Copy link
Copy Markdown
Contributor

810 commented Oct 9, 2014

@test I have tested it with kunena, it works.

Results without patch:

Page 1: forum/recent
Page 2: forum/recent?start=5

You can't navigate back to first page.

Results with patch:

Page 1: forum/recent?limitstart=0
Page 2: forum/recent?start=5

You can navigate on all pages

@Lavsteph
Copy link
Copy Markdown

@test it's all right for me with Kunena

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4488.

@Bakual Bakual added the RTC This Pull Request is Ready To Commit label Oct 10, 2014
@Bakual
Copy link
Copy Markdown
Contributor Author

Bakual commented Oct 10, 2014

Setting to RTC as we have three tests.

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Dec 8, 2014

This "fix" means, that now the first page of every list has 2 URLs. "whatever" and "whatever?limitstart=0". At the same time, limitstart was always renamed to start in our URLs. please revert this. It is not the solution to the problem. The problem is in the code in JModelList::populateState().

@Bakual
Copy link
Copy Markdown
Contributor Author

Bakual commented Dec 8, 2014

This "fix" is a revert of a regression. It reinstates what was there before the original PR (#3725) broke it. I'm not going to revert this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants