User tests: Successful: Unsuccessful:
Instead of removing the "limitstart" parameter in the router file (see #3725 ), we do not generate URLs with "limitstart=0".
Kind of weird to add it there to be then removed in the router, isn't it?
So components will be able to use the parameter "limitstart=0" if they want to... and we still avoid the duplicate url issue we used to have.
Sounds like a win / win! :)
Labels |
Added:
?
|
Yes I saw that this night.
Travis failed due to "JPaginationTest::testGetData", but at this moment I think there is something wrong the the "JPaginationTest" tests because with the modification of the router ( #3725 ) the unit tests should not work too.
Because we got a consistency problem.
On a side a unit test tell us that the parameter "limitstart=0" should be there so the pagination works. On the other end, the new router remove the parameter "limitstart=0" from the url so it's not possible to get it at all.
In my case on J!3.3.4 with "Use URL rewriting" disabled it fix the issue
Category | ⇒ | SEF |
It works with this fix but the address is now "?start=0"
not "?limitstart=0"
Hi,
Please precise your test context because if you use the classical pagination system, like in a article listing, you won't see any "limitstart=0" or "start=0" in the url.
But in your case, you tested in Kunena, which have his own pagination override for the generation of the url, so the modification in the pagination class does not affect you (and yes, that's one of the goal of this PR).
Nevertheless, there is still the modification in the router file and you just have to read the code to see that the "limistart" is always replaced by "start". Before the was the case when the value is superior than 0, in my modification it is done for every values.
But the problem I see in your message is the interrogation point.
Please be precise in your message too.
We are working in SEF context, so there is no "?" in the url.. It's a SEF url
You can't have the parameter "?start=0" because this variable in set in the router, if you don't use the router, you will got "?limitstart=0", if you have the router you will get "/start-0/".
So, your message is currently wrong even if the idea is partially right.
The SEF still change the display of the parameter "limitstart" into "start" but it does not mean that the variable "limitstart" is not available using JRequest.
I hope I am clear enough.
Regards,
No problem rich20 !
Now the question about Travis is still there (and I'm not an expert on the unit test files)
+1 the pagination works with this fix
No problem rich20 !
Now the question about Travis is still there (and I'm not an expert on the unit test files)
I have looked tests results, it doesn't expect a index.php?limitstart=0 for href for start :
JLIB_HTML_START
Actually it looks to me like the &limitstart=0
(or &start=0
) isn't optional and the other PR actually caused a regression.
The reason is that JModelList::populateState()
will not set the list.start state to 0 if the parameter is not present. It rather takes the value from the userstate instead. See https://github.com/joomla/joomla-cms/blob/staging/libraries/legacy/model/list.php#L594
So currently, extensions using that method can't get back to the first page if SEF is enabled.
That is likely also the reason why Travis rightfully fails with this PR, because you now try to apply the same change to non-SEF URLs as well.
The JPagination unit tests were pretty much the first unit tests I ever wrote so I wouldn't be suprised at all if the code quality there is lacking in some way!
Hi,
I'm not sure if I understand everything that is mentioned here. But I also have a problem with the pagination. The pagination link href URL contains 'start='. This does not work. It only works with the site settings 'URL rewriting' - Off'. When I change the 'start=' part of the URL to 'limitstart=' this shows the correct page. I do not know why. I'm currently on Joomla 3.3.6. But this problem also was there on 3.3.3 I believe.
My solution for now is to use jQuery to replace 'start=' part of the the pagination href URL's to 'limitstart='. This gets the pagination working for my site.
jQuery("div.pagination a").each(function() {
var value = jQuery(this).attr('href');
jQuery(this).attr('href', value.replace('start','limitstart'));
});
Hi,
This pull-request was mainly for the SEF problem related to the pagination and the lost of the "limitstart=0".
The fact that "limitstart" is replaced by "start" in the URL is made by the router and the router revert back the parameter so your problem is weird but it's not the exact same issue.
I hope that your issue will be solve soon ; I'll see to take a look next week and/or during the JBF.
Regards,
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-10-10 16:36:53 |
OK, thanks!
Travis fails.