Skip to content

Conversation

@laoneo
Copy link
Member

@laoneo laoneo commented May 18, 2018

Pull Request for Issue #20435.

Summary of Changes

Sends the onBeforeExecute correctly in the applications.

Testing Instructions

Enable the language filter plugin.

Expected result

No exception is thrown.

Actual result

Exception
Argument 1 passed to PlgSystemLanguageFilter::onBeforeExecute() must be an instance of Joomla\CMS\Event\BeforeExecuteEvent, instance of Joomla\Event\Event given
is thrown.

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 3fbe042


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20448.

@infograf768
Copy link
Member

@mbabker @wilsonge This one is rather urgent if we want to test associations and solve the multilingual issues

@roland-d
Copy link
Contributor

I have tested this item ✅ successfully on ac6b179

Before the fix after I activated the plugin it was impossible to access Joomla, after the fix everything worked as expected.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20448.

@ghost
Copy link

ghost commented May 19, 2018

@infograf768 can you please retest?

@rdeutz rdeutz merged commit 79fd949 into joomla:4.0-dev May 19, 2018
@laoneo laoneo deleted the j4/fix/event/trigger branch May 19, 2018 10:50
// Trigger the onBeforeExecute event
$this->getDispatcher()->dispatch(
'onBeforeExecute',
new BeforeExecuteEvent('onBeforeExecute', ['subject' => $this, 'container' => $this->getContainer()])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one doesn't work sadly :( The getContainer doesn't exist in this class

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a PR to fix it

@rdeutz rdeutz mentioned this pull request May 20, 2018
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.

6 participants