?

Failure

User tests: Successful: Unsuccessful:

avatar hikashop-jerome
hikashop-jerome
29 Sep 2014

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! :)

avatar hikashop-jerome hikashop-jerome - open - 29 Sep 2014
avatar jissues-bot jissues-bot - change - 29 Sep 2014
Labels Added: ?
avatar infograf768
infograf768 - comment - 30 Sep 2014

Travis fails.

avatar hikashop-jerome
hikashop-jerome - comment - 30 Sep 2014

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.

  1. the unit test should make some test with the SEF activated
  2. the unit test should test the pagination without "limitstart=0"

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.

avatar xillibit
xillibit - comment - 30 Sep 2014

In my case on J!3.3.4 with "Use URL rewriting" disabled it fix the issue

avatar brianteeman brianteeman - change - 1 Oct 2014
Category SEF
avatar rich20
rich20 - comment - 1 Oct 2014

It works with this fix but the address is now "?start=0"
not "?limitstart=0"

avatar hikashop-jerome
hikashop-jerome - comment - 1 Oct 2014

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,

avatar 810
810 - comment - 1 Oct 2014

@test
+1 - After apply fix. the pagination works again.

avatar rich20
rich20 - comment - 1 Oct 2014

@test
+1 - The pagination works again with this fix.

Excuse me, my first comment it was a misunderstanding by me.

avatar hikashop-jerome
hikashop-jerome - comment - 1 Oct 2014

No problem rich20 !

Now the question about Travis is still there (and I'm not an expert on the unit test files)

avatar Lavsteph
Lavsteph - comment - 1 Oct 2014

+1 the pagination works with this fix

avatar xillibit
xillibit - comment - 1 Oct 2014

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
avatar Bakual
Bakual - comment - 7 Oct 2014

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.

avatar wilsonge
wilsonge - comment - 8 Oct 2014

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!

avatar jjnxpct
jjnxpct - comment - 8 Oct 2014

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'));
});

avatar 810
810 - comment - 10 Oct 2014

I think this can be closed.

because f450b30
is merged.

avatar jjnxpct
jjnxpct - comment - 10 Oct 2014

Hi, I tried the f450b30 patch but this not solve the problem on my site (pagination on tags blogpage). I'll stick with the jQuery solution for now...

avatar hikashop-jerome
hikashop-jerome - comment - 10 Oct 2014

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,

avatar hikashop-jerome hikashop-jerome - close - 10 Oct 2014
avatar hikashop-jerome hikashop-jerome - change - 10 Oct 2014
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2014-10-10 16:36:53
avatar jjnxpct
jjnxpct - comment - 10 Oct 2014

OK, thanks!

Add a Comment

Login with GitHub to post a comment