#903110 by bojanz: Fixed Multiple pager support is partially broken. (with tests)

includes/pager.inc

@@ -82,17 +82,17 @@ public function execute() {
 
   /**
    * Ensure that there is an element associated with this query.
+   * If an element was not specified previously, then the value of the
+   * $maxElement counter is taken, after which the counter is incremented.
    *
-   * After running this query, access $this->element to get the element for this
+   * After running this method, access $this->element to get the element for this
    * query.
    */
   protected function ensureElement() {
-    if (!empty($this->element)) {
-      return;
-    }
-
+    if (!isset($this->element)) {
       $this->element = self::$maxElement++;
     }
+  }
 
   /**
    * Specify the count query object to use for this pager.
@@ -149,10 +149,20 @@ public function limit($limit = 10) {
    * whatever reason you want to explicitly define an element for a given query,
    * you may do so here.
    *
+   * Setting the element here also increments the static $maxElement counter,
+   * which is used for determining the $element when there's none specified.
+   *
+   * Note that no collision detection is done when setting an element ID
+   * explicitly, so it is possible for two pagers to end up using the same ID
+   * if both are set explicitly.
+   * 
    * @param $element
    */
   public function element($element) {
     $this->element = $element;
+    if ($element >= self::$maxElement) {
+      self::$maxElement = $element + 1;
+    }
     return $this;
   }
 }

modules/simpletest/tests/database_test.test

@@ -2216,6 +2216,43 @@ class DatabaseSelectPagerDefaultTestCase extends DatabaseTestCase {
       ->fetchCol();
     $this->assertEqual($ages, array('George', 'Ringo'), t('Pager query with having expression returned the correct ages.'));
   }
+  
+  /**
+  * Confirm that every pager gets a valid non-overlaping element ID.
+  */
+  function testElementNumbers() {
+    $_GET['page'] = '3, 2, 1, 0';
+    
+    $name = db_select('test', 't')->extend('PagerDefault')
+      ->element(2)
+      ->fields('t', array('name'))
+      ->orderBy('age')
+      ->limit(1)
+      ->execute()
+      ->fetchField();
+    $this->assertEqual($name, 'Paul', t('Pager query #1 with a specified element ID returned the correct results.'));
+    
+    // Setting an element smaller than the previous one
+    // should not overwrite the pager $maxElement with a smaller value.
+    $name = db_select('test', 't')->extend('PagerDefault')
+      ->element(1)
+      ->fields('t', array('name'))
+      ->orderBy('age')
+      ->limit(1)
+      ->execute()
+      ->fetchField();
+    $this->assertEqual($name, 'George', t('Pager query #2 with a specified element ID returned the correct results.'));
+  
+    $name = db_select('test', 't')->extend('PagerDefault')
+      ->fields('t', array('name'))
+      ->orderBy('age')
+      ->limit(1)
+      ->execute()
+      ->fetchField();
+    $this->assertEqual($name, 'John', t('Pager query #3 with a generated element ID returned the correct results.'));
+    
+    unset($_GET['page']);
+  }
 }