Simplify the event registration

[leofeyer]

See https://symfony.com/blog/new-in-symfony-4-4-simpler-event-listeners

[Toflar]

LGTM at first sight. Can you explain why some of them still require the event: kernel.request or event: kernel.response tag?

[leofeyer]

You mean something like this, right?

- { name: kernel.event_listener, event: kernel.request, priority: 255 }
- { name: kernel.event_listener, event: kernel.response, priority: -255 }

I don't think there is a way to assign two different priorities to two different events without keeping the event: key, is there?

[Toflar]

This for example, why is event: kernel.response still required here? There's no priority attribute.

[leofeyer]

Because otherwise Symfony throws an exception: Service "contao.listener.csrf_token_cookie" must define the "event" attribute on "kernel.event_listener" tags. 🤷‍♂

[Toflar]

Okay, but why? 😄

[leofeyer]

I don't know. But three remaining event: keys should not prevent us from merging this PR.

[Toflar]

I'm reviewing here and I would like to understand why it doesn't work so that we can either fix it or know why we have inconsistency now. Can you check which condition of RegisterListenersPass::getEventFromTypeDeclaration() is failing?

[leofeyer]

I have found the issue. The pass is looking for an __invoke() method and if there is none, you either have to specify the event or method key. If no method key is given, it is computed from the event key.

I guess it is more correct to specify the method names in our three cases (see 4a95e51).

[Toflar]

I guess it is more correct to specify the method names in our three cases (see 4a95e51).

I agree with that 👍

[leofeyer]

I went through all definitions with an event: key in their tags again and I was able to get rid of one more in a9e2e7f. The remaining ones cannot be removed without Symfony throwing an exception.