? Failure

User tests: Successful: Unsuccessful:

avatar remotehelp
remotehelp
5 Nov 2015

This patch remove "?limitstart=0" in pagination from "Page 1", "Back" and "Start" page.

avatar remotehelp remotehelp - open - 5 Nov 2015
avatar remotehelp remotehelp - change - 5 Nov 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 5 Nov 2015
Labels Added: ?
avatar Bakual
Bakual - comment - 5 Nov 2015

Last time we tried that, it failed horrible as you couldn't go back to page one.

Why do you want to remove that?

avatar remotehelp
remotehelp - comment - 5 Nov 2015

@Bakual I apply this patch on our site and this worked fine, "?limitstart=0" in pagination from "Page 1", "Back" and "Start" page be removed, - try this http://www.remoteshaman.com/blog?start=20 - and this http://www.remoteshaman.com/unix/gentoo/kak-ya-gentoo-linux-na-vmware-stavil?start=3

Worked fine.

"?limitstart=0" make dublcate first page. For example http://www.remoteshaman.com/blog and http://www.remoteshaman.com/blog?limitstart=0 - why?

I think "?limitstart=0" must be removed for best SEO.

avatar Bakual
Bakual - comment - 5 Nov 2015

For SEO, it doesn't matter at all actually.
For pages using SmartSearch or other places where the current page is stored in user session it makes it impossible to go back to page one.
Did you test in backend as well?

avatar remotehelp
remotehelp - comment - 6 Nov 2015

@Bakual in backend worked fine. "?limitstart=0" any way not need on first page and must be removed.

avatar zero-24
zero-24 - comment - 6 Nov 2015

@remotehelp Travis complains on codestyle and unit test issues see:

FILE: .../travis/build/joomla/joomla-cms/libraries/cms/pagination/pagination.php
--------------------------------------------------------------------------------
FOUND 16 ERROR(S) AFFECTING 14 LINE(S)
--------------------------------------------------------------------------------
 759 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if
     |       | (...)\n...{...}\n...else "
 769 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if
     |       | (...)\n...{...}\n...else "
 779 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if
     |       | (...)\n...{...}\n...else "
 789 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if(...)...{...}
     |       | ...else "
 790 | ERROR | Tabs must be used to indent lines; spaces are not allowed
 791 | ERROR | Tabs must be used to indent lines; spaces are not allowed
 791 | ERROR | Closing brace indented incorrectly; expected 3 spaces, found 17
 792 | ERROR | Tabs must be used to indent lines; spaces are not allowed
 793 | ERROR | Tabs must be used to indent lines; spaces are not allowed
 800 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if
     |       | (...)\n...{...}\n...else "
 823 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if (...)
     |       | ...{...} ...else "
 824 | ERROR | Tabs must be used to indent lines; spaces are not allowed
 825 | ERROR | Tabs must be used to indent lines; spaces are not allowed
 825 | ERROR | Closing brace indented incorrectly; expected 4 spaces, found 18
 826 | ERROR | Tabs must be used to indent lines; spaces are not allowed
 827 | ERROR | Tabs must be used to indent lines; spaces are not allowed
--------------------------------------------------------------------------------
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
--------------------------------------------------------------------------------

and

There were 3 failures:
1) JPaginationTest::testGetData with data set #0 (100, 40, 20, 3, array(array('JLIB_HTML_VIEW_ALL', '0', 'index.php', '', ''), array('JLIB_HTML_START', '0', 'index.php?limitstart=0', '', ''), array('JPREV', '20', 'index.php?limitstart=20', '', ''), array('JNEXT', '60', 'index.php?limitstart=60', '', ''), array('JLIB_HTML_END', '80', 'index.php?limitstart=80', '', ''), array('3', '', null, '', true)))
This is not the expected start
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'text' => 'JLIB_HTML_START'
     'base' => '0'
-    'link' => 'index.php'
+    'link' => 'index.php?limitstart=0'
     'prefix' => ''
-    'active' => false
+    'active' => ''
 )
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/cms/pagination/JPaginationTest.php:341
2) JPaginationTest::testGetPagesLinks with data set #0 (100, 50, 20, '<ul><li class="pagination-sta...></ul>')
The expected output of the pagination is incorrect
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<ul><li class="pagination-start"><a title="JLIB_HTML_START" href="index.php" class="hasTooltip pagenav">JLIB_HTML_START</a></li><li class="pagination-prev"><a title="JPREV" href="index.php?limitstart=20" class="hasTooltip pagenav">JPREV</a></li><li><a href="index.php" class="pagenav">1</a></li><li><a href="index.php?limitstart=20" class="pagenav">2</a></li><li><span class="pagenav">3</span></li><li><a href="index.php?limitstart=60" class="pagenav">4</a></li><li><a href="index.php?limitstart=80" class="pagenav">5</a></li><li class="pagination-next"><a title="JNEXT" href="index.php?limitstart=60" class="hasTooltip pagenav">JNEXT</a></li><li class="pagination-end"><a title="JLIB_HTML_END" href="index.php?limitstart=80" class="hasTooltip pagenav">JLIB_HTML_END</a></li></ul>'
+'<ul><li class="pagination-start"><a title="JLIB_HTML_START" href="index.php?limitstart=0" class="hasTooltip pagenav">JLIB_HTML_START</a></li><li class="pagination-prev"><a title="JPREV" href="index.php?limitstart=20" class="hasTooltip pagenav">JPREV</a></li><li><a href="index.php?limitstart=0" class="pagenav">1</a></li><li><a href="index.php?limitstart=20" class="pagenav">2</a></li><li><span class="pagenav">3</span></li><li><a href="index.php?limitstart=60" class="pagenav">4</a></li><li><a href="index.php?limitstart=80" class="pagenav">5</a></li><li class="pagination-next"><a title="JNEXT" href="index.php?limitstart=60" class="hasTooltip pagenav">JNEXT</a></li><li class="pagination-end"><a title="JLIB_HTML_END" href="index.php?limitstart=80" class="hasTooltip pagenav">JLIB_HTML_END</a></li></ul>'
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/cms/pagination/JPaginationTest.php:450
3) JPaginationTest::testBuildDataObject with data set #0 (100, 40, 20, 3, array(array('JLIB_HTML_VIEW_ALL', '0', 'index.php', '', ''), array('JLIB_HTML_START', '0', 'index.php?limitstart=0', '', ''), array('JPREV', '20', 'index.php?limitstart=20', '', ''), array('JNEXT', '60', 'index.php?limitstart=60', '', ''), array('JLIB_HTML_END', '80', 'index.php?limitstart=80', '', ''), array('3', '', null, '', true)))
This is not the expected start
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'text' => 'JLIB_HTML_START'
     'base' => '0'
-    'link' => 'index.php'
+    'link' => 'index.php?limitstart=0'
     'prefix' => ''
-    'active' => false
+    'active' => ''
 )
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/cms/pagination/JPaginationTest.php:779

https://travis-ci.org/joomla/joomla-cms/jobs/89386273

avatar remotehelp
remotehelp - comment - 6 Nov 2015

@zero-24
"PHPUnit 4.8.11" this not latest version!
see: https://github.com/sebastianbergmann/phpunit

"PHP_CODESNIFFER" maybe also not latest version!

"'active' => ''" instead "'active' => false" and "'link' => 'index.php?limitstart=0'" instead "'link' => 'index.php'" - this a poor recomendation

Now this is a travis problems, but not joomla cms where this patch worked fine.

avatar zero-24
zero-24 - comment - 6 Nov 2015

@remotehelp this is a known problem if you whant to help to bring it up to date see: https://github.com/joomla/coding-standards Thanks.

avatar Bakual
Bakual - comment - 6 Nov 2015

For reference the last time we tried it:
PR #3725
Revert: #4488

Another PR which got closed: #4393

Thus I really doubtthis will work now :)

avatar remotehelp
remotehelp - comment - 7 Nov 2015

@Bakual PR #3725, Revert: #4488 and #4393 - this is bullshit :)

This #8280 really work now - checked up in practice!

Check for yourself.

avatar zero-24 zero-24 - change - 7 Nov 2015
Category Libraries
avatar Bakual
Bakual - comment - 9 Nov 2015

Same issue as with #3725 which got reverted because it broke extensions.
The reason is that some extension store the currently visited page in the userstate.
Similar to what is done in backend where you can go to another menu item and come back and still are on the same article list page.
I don't think we use that behavior in core in frontend, but there are extensions which do. Removing that parameter means you can't go back to page 1 because if the parameter isn't present the code takes the value from the userstate instead.
In backend it still works imho because the pagination is stored in the form, and not as part of the URL.

Closing as this doesn't work.

avatar Bakual Bakual - change - 9 Nov 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-11-09 19:45:49
Closed_By Bakual
avatar Bakual Bakual - close - 9 Nov 2015
avatar Bakual Bakual - close - 9 Nov 2015
avatar remotehelp
remotehelp - comment - 10 Nov 2015

@Bakual you stupid liar! Everything works fine, but if some extension not work with this, then this problem in that extension.

Mind enough to make shit-codes, but correct on already finished can not.

Bye stupid mainteiner's.

avatar mbabker mbabker - locked - 10 Nov 15

Add a Comment

Login with GitHub to post a comment