User tests: Successful: Unsuccessful:
This is a redo of #3630, using the current staging branch and with some improvements.
This PR implements the following plugin events for plugins (group: content):
Event | Views |
---|---|
onContentPrepare |
|
onContentAfterTitle |
|
onContentBeforeDisplay |
|
onContentAfterDisplay |
|
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Category | ⇒ | Components Plugins |
Thankyou :) I'll try and find some time to test over the next week or so. But can't guarantee anything :(
Easy | No | ⇒ | Yes |
Testing works!
I have tested this item successfully on ccb17b0
@test successfully tested.
I have tested this item successfully on ccb17b0
@Test works fine
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
@Harmageddon can you fix the merge conflicts.
Milestone |
Added: |
This PR has received new commits.
CC: @KlausKrippeit, @Rivaner, @yvesh
Thanks @Harmageddon
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-10-26 19:50:34 |
Closed_By | ⇒ | roland-d |
Labels |
Removed:
?
|
This pull request has introduced two regressions:
1) Via 88fb40b#diff-075095e768ef950cf2dcc5f08cf5436aR148 plugin events are now being triggered twice for all items on the com_content.category
view
2) 88fb40b#diff-075095e768ef950cf2dcc5f08cf5436aR159 is hardcoded to com_contact.category
as the context instead of correctly determining the extension as is done with every other trigger
Can you open a new issue (or even PR) that references to this PR?
If I do it's taking the event triggers out of JViewCategory
because it's damn near a B/C issue (extensions extending it and calling that method are now either getting new plugin events that weren't in place originally or in the case of com_content now causes events to be triggered twice).
This pull request should not have been merged without testing against ALL components because of the changes in JViewCategory at a minimum. Also, the actual merge for this pull request (commit 88fb40b) is so vastly different than the state this pull request was accepted in at this point I would practically demand that the original merge be reverted and the merge attempted again without all the extra changes (compare the changes for components/com_contact/views/contact/tmpl/default.php from this pull request with the committed changes. The diffs for these files alone do not align, not to mention the number of files changed in this pull request compared to the actual merge.
Any update on best way to fix the double event triggering ?
Also 3 out of the 4 triggers are using ->core_params which does not exist
and 1 is using ->params that does exist
https://github.com/joomla/joomla-cms/blob/staging/libraries/legacy/view/category.php#L161-L168
$results = $dispatcher->trigger('onContentAfterTitle', array($this->extension . '.category', &$itemElement, &$itemElement->core_params, 0));
...
$results = $dispatcher->trigger('onContentBeforeDisplay', array($this->extension . '.category', &$itemElement, &$itemElement->core_params, 0));
...
$results = $dispatcher->trigger('onContentAfterDisplay', array($this->extension . '.category', &$itemElement, &$itemElement->core_params, 0));
...
Regarding the empty parameters, there is a report for it here:
#8963
ok, my comment about 3rd party extensions is mostly wrong, only a very small number (if any) will need to be updated
because function commonCategoryDisplay which got the new code to trigger plugin event is inside JViewCategory and not inside JViewLegacy, and then double triggering is only if you call parent::commonCategoryDisplay();
I'm so sorry I forgot about this. I'm out today but will try and get back on this either this evening or tomorrow. Please ping me repeatedly on this if it feels like I've forgotten about the issue!
Besides fixing $itemElement->core_params (which does not exist) to be ->params
A simple and clean fix for the double category triggering is
public function commonCategoryDisplay()
at libraries/legacy/view/category.php (class JViewCategory) so as to be:
public function commonCategoryDisplay($trigger_events=false)
Then update views that extend JCategoryView (and that also call commonCategoryDisplay) and for which default code for triggering events is good enough:
com_weblinks\views\category\view.html.php
com_newsfeed\views\category\view.html.php
com_contact\views\category\view.html.php
parent::commonCategoryDisplay($trigger_events=true);
And then views that need a custom preparation for plugin event triggering and want to do it inside their own code:
com_content\views\category\view.html.php
parent::commonCategoryDisplay($trigger_events=false);
Also since plugin event trigger was added to the parent class (JViewCategory),
otherwise it is rather inflexible, and any view wanting to do some extra preparation for plugin event triggering will need to skip the commonCategoryDisplay altogether and copy all its code inside the current view
plus it becomes background compatible
Testing on latest staging double event triggering is still an issue see backbutton/content-plugin#6
Thankyou! I have left this up on my laptop and will have code to fix it on the way home from work this evening!
Updated the PR with Beez support and a slightly different context for category descriptions.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8024.