MDL-39801 navigation_node::remove fails if first child does not have a key

This commit is contained in:
Marina Glancy 2013-05-31 11:45:47 +10:00
parent 73f560c7b2
commit e3b11a6e2b
2 changed files with 9 additions and 1 deletions

View File

@ -899,7 +899,7 @@ class navigation_node_collection implements IteratorAggregate {
$child = $this->get($key, $type); $child = $this->get($key, $type);
if ($child !== false) { if ($child !== false) {
foreach ($this->collection as $colkey => $node) { foreach ($this->collection as $colkey => $node) {
if ($node->key == $key && $node->type == $type) { if ($node->key === $key && $node->type == $type) {
unset($this->collection[$colkey]); unset($this->collection[$colkey]);
break; break;
} }

View File

@ -60,6 +60,8 @@ class navigation_node_testcase extends basic_testcase {
$this->node = new navigation_node('Test Node'); $this->node = new navigation_node('Test Node');
$this->node->type = navigation_node::TYPE_SYSTEM; $this->node->type = navigation_node::TYPE_SYSTEM;
// We add the first child without key. This way we make sure all keys search by comparision is performed using ===
$this->node->add('first child without key', null, navigation_node::TYPE_CUSTOM);
$demo1 = $this->node->add('demo1', $this->inactiveurl, navigation_node::TYPE_COURSE, null, 'demo1', new pix_icon('i/course', '')); $demo1 = $this->node->add('demo1', $this->inactiveurl, navigation_node::TYPE_COURSE, null, 'demo1', new pix_icon('i/course', ''));
$demo2 = $this->node->add('demo2', $this->inactiveurl, navigation_node::TYPE_COURSE, null, 'demo2', new pix_icon('i/course', '')); $demo2 = $this->node->add('demo2', $this->inactiveurl, navigation_node::TYPE_COURSE, null, 'demo2', new pix_icon('i/course', ''));
$demo3 = $this->node->add('demo3', $this->inactiveurl, navigation_node::TYPE_CATEGORY, null, 'demo3',new pix_icon('i/course', '')); $demo3 = $this->node->add('demo3', $this->inactiveurl, navigation_node::TYPE_CATEGORY, null, 'demo3',new pix_icon('i/course', ''));
@ -251,8 +253,14 @@ class navigation_node_testcase extends basic_testcase {
$this->assertInstanceOf('navigation_node', $this->node->get('remove2')); $this->assertInstanceOf('navigation_node', $this->node->get('remove2'));
$this->assertInstanceOf('navigation_node', $remove2->get('remove3')); $this->assertInstanceOf('navigation_node', $remove2->get('remove3'));
// Remove element and make sure this is no longer a child.
$this->assertTrue($remove1->remove()); $this->assertTrue($remove1->remove());
$this->assertFalse($this->node->get('remove1'));
$this->assertFalse(in_array('remove1', $this->node->get_children_key_list(), true));
// Remove more elements
$this->assertTrue($this->node->get('remove2')->remove()); $this->assertTrue($this->node->get('remove2')->remove());
$this->assertFalse($this->node->get('remove2'));
$this->assertTrue($remove2->get('remove3')->remove()); $this->assertTrue($remove2->get('remove3')->remove());
$this->assertFalse($this->node->get('remove1')); $this->assertFalse($this->node->get('remove1'));