[#41497] - [5.0][Events] Fix backward compatibility for onAfterRenderModules
- Fixed in Code Base
- 2 Sep 2023
- Medium
- Build: 5.0-dev
- # 41497
- Diff
- Fedik:event-mod-bc
User tests: Successful: Unsuccessful:
Summary of Changes
Fix backward compatibility for onAfterRenderModules
Testing Instructions
Add following listener to Sef system plugin:
public function onAfterRenderModules(&$value) {
$value .= ' aaaa';
}Open the site, and check rendered modules
Actual result BEFORE applying this Pull Request
Nothing changed
Expected result AFTER applying this Pull Request
Every module position now have 'aaaa' appended
Link to documentations
Please select:
- Documentation link for docs.joomla.org:
- No documentation changes for docs.joomla.org needed
- Pull Request link for manual.joomla.org: joomla/Manual#177
- No documentation changes for manual.joomla.org needed
Reference:
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Status | New | ⇒ | Pending |
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-09-02 14:22:23 |
| Closed_By | ⇒ | HLeithner | |
| Labels |
Added:
bug
PR-5.0-dev
|
||
Additional ArrayProxy is nothing we plan to have for ever right? because it only hides the & reference
Yeah, it for replacement of array with reference.
There many events that use "array" editing.
Tricky thing.
Also I think we should convert the array back after we triggered the event with them?
The changes applied to original array, converting it back is unnecessary.
Sure but the thing is we don't want that the Plugin manipulate the array directly?
Yes, that the idea. because it hard to debug where the data gets modified by reference.
With ArrayProxy it at least possible to attach debugger to ArrayProxy::offsetSet().
We have around 15-20 events using array modification.
Unfortunately, there also events where we mix object/array for $data,
we should make this consistent... maybe not in 5 but later
TBH, I have no idea how it can be done, for now and in future.
Maybe in one of next beta, I will add updateData() method, to all events with $data.
This will enforce developers to use $event->updateData($data);, instead of editing $data "in place", to prepare them for future.
Similar what we made for strings $downloadEvent->updateUrl($url);
Need to think.
We have $event->setData() don't we have this to update arguments? (when not immutable?)
$event->setData()
It is part of argument validation, it cannot be public, or you get a loop. (Do not ask why)
joomla-cms/libraries/src/Event/AbstractEvent.php
Lines 202 to 208 in a78c8f5
Merging this as with code review and based on the other similar PRs, before beta1 to get enough feedback for final release.
Additional ArrayProxy is nothing we plan to have for ever right? because it only hides the & reference, Also I think we should convert the array back after we triggered the event with them?