Bug: pager "prev" and "next" links pointing to wrong URIs

[korgoth]

While paginating a model i noticed that when rendered my "prev" and "next" links point to the wrong URIs.
The result has 116 entries and they are limited to 20 per page using:

$this->galleries->orderBy('id', 'desc')->paginate(20);

And then simply displaying it using:

$this->galleries->pager->links();

This results in the following HTML being rendered when i am on Page 1:

<ul class="pagination">
		
	<li class="active">
		<a href="http://localhost:8080/chrono?page=1">1</a>
	</li>
	<li>
		<a href="http://localhost:8080/chrono?page=2">2</a>
	</li>
	<li>
		<a href="http://localhost:8080/chrono?page=3">3</a>
	</li>
	<li>
		<a href="http://localhost:8080/chrono?page=4" aria-label="Next">
			<span aria-hidden="true">»</span>
		</a>
	</li>
	<li>
		<a href="http://localhost:8080/chrono?page=6" aria-label="Last">
			<span aria-hidden="true">Last</span>
		</a>
	</li>
</ul>

Now i noticed that while on page number 1, the "next" link pointed to page number 4. Similarly on page 2 - next pointed to page 5, on page 3 next pointed to 6.
The "prev" button does exactly the same - with the same offset of 3 pages.

I tried digging up the CI code and noticed that the problem seems to be in the PageRenderer class. In that class the functions getPrevious() and getNext() seem to be referring and modifying the $this->first and $this->last property, as i think they should be modifying the $this->current

Current code in system/Pager/PagerRenderer.php starting from line 36 is:

public function getPrevious()
{
	if (! $this->hasPrevious())
	{
		return null;
	}

	$uri = clone $this->uri;

	if ($this->segment === 0)
	{
		$uri->addQuery($this->pageSelector, $this->first - 1);
	}
	else
	{
		$uri->setSegment($this->segment, $this->first - 1);
	}

	return (string) $uri;
}
...
public function getNext()
{
	if (! $this->hasNext())
	{
		return null;
	}

	$uri = clone $this->uri;

	if ($this->segment === 0)
	{
		$uri->addQuery($this->pageSelector, $this->last + 1);
	}
	else
	{
		$uri->setSegment($this->segment, $this->last + 1);
	}

	return (string) $uri;
}

I think it should be:

public function getPrevious()
{
	if (! $this->hasPrevious())
	{
		return null;
	}

	$uri = clone $this->uri;

	if ($this->segment === 0)
	{
		$uri->addQuery($this->pageSelector, $this->current - 1);
	}
	else
	{
		$uri->setSegment($this->segment, $this->current - 1);
	}

	return (string) $uri;
}
...
public function getNext()
{
	if (! $this->hasNext())
	{
		return null;
	}

	$uri = clone $this->uri;

	if ($this->segment === 0)
	{
		$uri->addQuery($this->pageSelector, $this->current + 1);
	}
	else
	{
		$uri->setSegment($this->segment, $this->current + 1);
	}

	return (string) $uri;
}

CI Version: 4.0.2
Affected modules: Pager
Expected behavior: Pager "next" link to lead to the next page, and not to the next "set of non-displayed page links (due to surround links limit) "

[nyufeng]

You seem to have found a solution, Can you try to fix ?

[korgoth]

I would love to, but i first want to confirm that this is an expected behavior since the comments on those functions state something else. Both functions say:

The previous page is NOT the
 * page before the current page, but is the page just before the
 * "first" page.

I dont know why this is the expected behavior and whether or not i should change it?

[MGatner]

This is expected behavior. “Previous” and “Next” do not refer to the pages but to the row of links. The documentation isn’t super clear but read up on the pagination methods: https://codeigniter4.github.io/userguide/libraries/pagination.html#creating-the-view

[MGatner]

So in this example links to page 8-12 are displayed and the next arrow would take you to the next row of links after the current page - eg 11-15 (because 13 would be the active page).

[korgoth]

Ok, but how is it rational to not have a "next page" and "previous page" buttons in a pagination? I see there are methods about that - maybe i should just write my own view, to get rid of this.
I admit it is not a bug, since it was intentional (and thats indeed in the docs) but i believe in any normal pagination system those buttons are a given

[jlamim]

I applied a PR that fits this, since it doesn't make much sense for "next" and "prev" to be for a paging group when in fact the global understanding of pagination means that "next" and "prev" are linked directly to pages .

[durbintl]

I think this feature or bug could be useful when jumping through huge data sets, so I don't think this should be removed. It definitely needs to be renamed though. From a front end user stand point there needs to be a way to just increment/decrement the page without setting setSurroundCount(0) eliminating most of the point of pagination. Even W3C states this basic functionality, although quite ambiguously, at https://www.w3.org/dpub/IG/wiki/Pagination_Requirements#Ability_to_select_page.28s.29_for_display

Currently I'm using what @korgoth suggested and have been doing so since release.

[durbintl]

@lonnieezell I noticed that in the v4.0.3 release that you stated that this issue has been fixed, however when I download a copy of the ZIP or Tar file the code doesn't reflect this. When compared with the commit for that release the code does reflect the changes in the commit, but not the download. Also the develop branch does not reflect the change to pagination either. I haven't compared all the fixes in the release, so there may be other changes that didn't make it to the release. Am I missing something or does it appear to anyone else that the code in the download mostly reflects the current develop branch.

[MGatner]

@durbintl I see the updated code in the release commit to codeigniter/framework: codeigniter4/framework@edd88b1#diff-caa61ed5cfc1c6799db64ce075139b6c

When I download it I have the same code. Can you specify where you are going to download?

[durbintl]

@MGatner I'm pulling from multiple sources with the same results as noted below.

• From Github @ https://github.com/codeigniter4/CodeIgniter4/releases/tag/v4.0.3 using the Assets links at the bottom.
• Directly from CI.com which is pulled from https://github.com/codeigniter4/framework/archive/v4.0.3.zip
• Using Composer's update function.

To ensure I don't have any heavy caching going on for some reason I spun up a VM and double checked.

All yield the same results, which do not seem to be the same as the commit you provided. The file I am specifically looking at as comparison is at system/Pager/PagerRenderer.php codeigniter4/framework@edd88b1#diff-a49d9d6f18fe7684d344c5f7ecf34e1b

[michalsn]

I checked again and everything seems fine.