Skip to content

Commit

Permalink
Fixed faulty loop management when unsubscribing listeners
Browse files Browse the repository at this point in the history
[Closes #90]
  • Loading branch information
kytart authored and fprochazka committed May 13, 2017
1 parent fb88e44 commit 2a7dfe4
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/Kdyby/Events/EventManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,16 @@ public function removeEventListener($unsubscribe, $subscriber = NULL)
foreach ($this->listeners[$eventName] as $priority => $listeners) {
foreach ($listeners as $k => $listener) {
if (!($listener == $subscriber || (is_array($listener) && $listener[0] == $subscriber))) {
continue(2);
continue;
}
$key = $k;
break;
}

if (!isset($key)) {
continue;
}

unset($this->listeners[$eventName][$priority][$key]);
if (empty($this->listeners[$eventName][$priority])) {
unset($this->listeners[$eventName][$priority]);
Expand All @@ -237,6 +241,7 @@ public function removeEventListener($unsubscribe, $subscriber = NULL)
unset($this->sorted[$eventName]);
}

unset($key);

This comment has been minimized.

Copy link
@Majkl578

Majkl578 May 13, 2017

Member

I'd rather declare $key = null at the beginning of the scope.

This comment has been minimized.

Copy link
@fprochazka

fprochazka May 13, 2017

Member

That's a good point, thank you!

This comment has been minimized.

Copy link
@fprochazka

fprochazka May 13, 2017

Member
}
}
}
Expand Down
15 changes: 15 additions & 0 deletions tests/KdybyTests/Events/EventManager.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,21 @@ class EventManagerTest extends Tester\TestCase
}



public function testRemovingListenersIssue90()
{
$listener1 = new EventListenerMock();
$listener2 = new EventListenerMock2();
$this->manager->addEventListener('onFoo', $listener1);
$this->manager->addEventListener('onFoo', $listener2);
Assert::count(2, $this->manager->getListeners('onFoo'));

$this->manager->removeEventListener('onFoo', $listener2);
Assert::count(1, $this->manager->getListeners('onFoo'));
Assert::same(['onFoo' => [$listener1]], $this->manager->getListeners());
}


public function testListenerDontHaveRequiredMethodException()
{
$evm = $this->manager;
Expand Down

0 comments on commit 2a7dfe4

Please sign in to comment.