I will look at this.

Hi Martin, your implementation breaks the option to check to recursively:

/**
 * @group ZF-11664
 */
public function testIsActiveWithoutAndWithRecursiveOption()
{
    // Parent
    $page = new Zend_Navigation_Page_Mvc(array(
        'controller' => 'index',
        'action'     => 'index',
    ));

    // Child
    $page->addPage(new Zend_Navigation_Page_Mvc(array(
        'controller' => 'index',
        'action'     => 'foo',
    )));

    // Front controller
    $this->_front->getRequest()->setParams(array(
        'controller' => 'index',
        'action'     => 'foo'
    ));

    $this->assertFalse($page->isActive());

    $this->assertTrue($page->isActive(true));
}

There was 1 failure:

1) Zend_Navigation_Page_MvcTest::testIsActiveWithoutAndWithRecursiveOption
Failed asserting that  is true.

I think it is because my code could return NULL which is then compared in unit test with === to FALSE. Following code will always return TRUE or FALSE.

<?php
class XTZend_Navigation_Page_Mvc extends Zend_Navigation_Page_Mvc
{
    protected $_active = null;

    public function isActive($recursive = false)
    {
        if (is_null($this->_active)) {
            return (parent::isActive($recursive)===true);
        }

        return ($this->_active===true);
    }
}

Now ``` is never "false" and the all the code from Zend_Navigation_Page_Mvc::isActive() executed. Your fix brings no changes to the old behavior.

sorry, I don't understand what you mean. Does it not fix the "passing test" issue or you mean that it is not changing the fact that current Zend_Navigation_Page accepts only $page->setActive(true), but ignores $page->setActive(false);

Maybe I explained it not good enough, but my suggested code was/is only proposal how to change it, the main point is that $page->setActive(false); should work the same way as $page->setActive(true);. If it was set manualy it should not execute all the detection code in Zend_Navigation_Page_Mvc.

current implementation:

$page->setActive(true);
$b = $page->isActive(); // always TRUE, evaluation code in Zend_Navigation_Page_Mvc not executed

// but
$page->setActive(false);
$b = $page->isActive(); // you don't know the result, it will always execute the evaluation code in Zend_Navigation_Page_Mvc

my suggestion:

$page->setActive(true);
$b = $page->isActive(); // always TRUE, evaluation code in Zend_Navigation_Page_Mvc not executed

// but
$page->setActive(false);
$b = $page->isActive(); // always FALSE, , evaluation code in Zend_Navigation_Page_Mvc not executed

// and
$page->setActive(null); // the default behaviour, because $_active = null;
$b = $page->isActive(); // you don't know the result, it will execute the evaluation code in Zend_Navigation_Page_Mvc

I am not sure what exactly should be returned if $page->isActive(true), but it is more natural to return TRUE/FALSE if it was set manualy without to cascade. If the programmer wants to cascade, he should not set it manualy with setActive();

Hi Martin, thanks for your explanation. Here is another unit test:

/**
 * @group ZF-11664
 */
public function testSetActiveAndIsActive()
{
    // Page
    $page = new Zend_Navigation_Page_Mvc(array(
        'controller' => 'foo',
        'action'     => 'bar',
    ));

    // Front controller
    $this->_front->getRequest()->setParams(array(
        'controller' => 'foo',
        'action'     => 'bar'
    ));

    $this->assertTrue($page->isActive()); // Result: TRUE

    $page->setActive(false);
    $this->assertFalse($page->isActive()); // Result: TRUE
}

There were 1 failures:

1) Zend_Navigation_Page_MvcTest::testSetActiveAndIsActive
Failed asserting that  is false.

Fix and unit tests added.

Fixed in trunk (1.12.0): r24857