Skip to content

fix(core): restore event context in nested invokeEvent calls#16892

Open
Ibochkarev wants to merge 1 commit intomodxcms:3.xfrom
Ibochkarev:fix/14063-event-name-static-plugin
Open

fix(core): restore event context in nested invokeEvent calls#16892
Ibochkarev wants to merge 1 commit intomodxcms:3.xfrom
Ibochkarev:fix/14063-event-name-static-plugin

Conversation

@Ibochkarev
Copy link
Collaborator

What does it do?

Save and restore $this->event in invokeEvent() via try/finally so that outer plugins see the correct event name when nested invokeEvent calls occur (e.g. OnMediaSourceGetProperties during page load).

Why is it needed?

When a static plugin calls $modx->invokeEvent() from within another event handler, the nested call overwrites $this->event. After the nested call returns, the outer plugin sees the inner event's name instead of its own.

How to test

  1. Create a plugin on OnWebPageInit that prints $modx->event->name.
  2. Create a Media Source that triggers OnMediaSourceGetProperties during page load.
  3. Load a page; the plugin should show OnWebPageInit, not OnMediaSourceGetProperties.

Related issue(s)/PR(s)

Resolves #14063

Save and restore $this->event in invokeEvent() via try/finally so that
outer plugins see the correct event name when nested invokeEvent calls
occur (e.g. OnMediaSourceGetProperties during page load).

Resolves modxcms#14063
@Ibochkarev Ibochkarev force-pushed the fix/14063-event-name-static-plugin branch from d5001bc to c46b1fd Compare February 25, 2026 02:31
@biz87
Copy link

biz87 commented Feb 25, 2026

Code Review

Summary

Fixes modX::invokeEvent() to save and restore $this->event using try/finally, preventing nested invokeEvent() calls from corrupting the outer event context. Previously, when a plugin triggered another event (e.g. OnMediaSourceGetProperties during OnWebPageInit), the inner call overwrote $this->event and the outer plugin would see the wrong event name after the nested call returned.

Assessment

Clean, correct fix. The try/finally pattern is the right approach — it guarantees $this->event is restored even if an exception is thrown during plugin execution. The return $results inside the try block works correctly with finally in PHP. The formatting changes (spacing normalization, multi-line getObject call) are minor and consistent with PSR-12. No behavioral side effects — the method returns the same results, it just properly restores state afterward.

Verdict: Approve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect event name when calling static plugin

2 participants