Development

#3562 (sfPager::setPage(null) bug?)

You must first sign up to be able to contribute.

Ticket #3562 (closed defect: fixed)

Opened 7 months ago

Last modified 6 months ago

sfPager::setPage(null) bug?

Reported by: ryer Assigned to: fabien
Priority: minor Milestone: 1.0.17
Component: other Version: 1.0.16
Keywords: addon propel pager page Cc:
Qualification: Unreviewed

Description 

$pager = sfPropelPager($crit, $limit);
$pager->setPage(null);

Notice: Undefined offset: -1 in /usr/share/pear/symfony/addon/sfPager.class.php on line 88

cause?
http://trac.symfony-project.com/changeset/8680

Change History

05/16/08 11:41:20 changed by FabianLange

  • status changed from new to closed.
  • resolution set to worksforme.

why are you doing ->setPage(null)?

my change as such is correct. The -1 offset doesnt matter because getLinks whould never be invoked in page 0 case, which is the internal setting for: show all entries on one page.

if your pager has more than 1 page then using setPage(null) makes no sense. The first page in multi pages is setPage(1). Or leave out the setPage(null) completely

05/22/08 10:31:06 changed by phennim

Since this patch breaks BC, and setPage(0) is/was undocumented behavior (for symfony), perhaps a better warning should have been issued.

Before you could call $p->setPage($this->getRequestParameter('page')) and still get the first page if no page param was set.

05/22/08 10:49:13 changed by FabianLange

(In [9179]) restored undocumented way to pass null to setPage to select the first page. Enhanced so that it works in conjunction to maxPerPage. refs #3562

05/22/08 10:51:35 changed by FabianLange

  • status changed from closed to reopened.
  • resolution deleted.
  • milestone set to 1.0.17.

05/22/08 10:51:53 changed by FabianLange

  • status changed from reopened to closed.
  • resolution set to fixed.

phennim you are right that this is an acceptable way to set the Page, even when it was not documented. I commited a patch in r9179 that will enable you again doing so. I hope that this suites your needs

05/22/08 11:26:45 changed by phennim

That's a speedy reply Fabian.

If you fix $this->getMaxPerPage(), then it looks good to me (assuming setPage(0) can only be called if maxPerPage=0).

05/22/08 11:40:06 changed by FabianLange

(In [9182]) fixed my last commit, seems that i wasn't fully awake yet. refs #3562