From 337c63aac40f14389009bd6632eee4d2844dd29b Mon Sep 17 00:00:00 2001
From: Marina Glancy <marina@moodle.com>
Date: Fri, 31 May 2013 11:45:47 +1000
Subject: [PATCH] MDL-39801 navigation_node::remove fails if first child does
 not have a key

---
 lib/navigationlib.php            | 2 +-
 lib/tests/navigationlib_test.php | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/navigationlib.php b/lib/navigationlib.php
index 72ab3691e4a..9266975b143 100644
--- a/lib/navigationlib.php
+++ b/lib/navigationlib.php
@@ -889,7 +889,7 @@ class navigation_node_collection implements IteratorAggregate {
         $child = $this->get($key, $type);
         if ($child !== false) {
             foreach ($this->collection as $colkey => $node) {
-                if ($node->key == $key && $node->type == $type) {
+                if ($node->key === $key && $node->type == $type) {
                     unset($this->collection[$colkey]);
                     break;
                 }
diff --git a/lib/tests/navigationlib_test.php b/lib/tests/navigationlib_test.php
index 63804f746d6..1c046ceb40a 100644
--- a/lib/tests/navigationlib_test.php
+++ b/lib/tests/navigationlib_test.php
@@ -60,6 +60,8 @@ class navigation_node_testcase extends basic_testcase {
 
         $this->node = new navigation_node('Test Node');
         $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', ''));
         $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', ''));
@@ -251,8 +253,14 @@ class navigation_node_testcase extends basic_testcase {
         $this->assertInstanceOf('navigation_node', $this->node->get('remove2'));
         $this->assertInstanceOf('navigation_node', $remove2->get('remove3'));
 
+        // Remove element and make sure this is no longer a child.
         $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->assertFalse($this->node->get('remove2'));
         $this->assertTrue($remove2->get('remove3')->remove());
 
         $this->assertFalse($this->node->get('remove1'));
-- 
2.11.4.GIT