joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.69k stars 3.63k forks source link

Article View that was opened via a link from Article Category List: Module loaded twice #18016

Closed sandewt closed 6 years ago

sandewt commented 6 years ago

Steps to reproduce the issue

If you open an article, wich is part of an Article Category List and in wich a module is loaded with loadposition, the module is loaded twice. This can break (custom) code in Joomla! 3.8.0.

Expected result

Actual result

System information (as much as possible)

Additional comments

See image: example J3.8.0

debug

Attention to: (2 x 2) Application: beforeRenderModule mod_users_latest (Users Latest (copy)) Application: afterRenderModule mod_users_latest (Users Latest (copy))

In Joomla! 3.7.5 the module is loaded once. (OK)

ghost commented 6 years ago

looks similar to #17999


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

ggppdk commented 6 years ago

Reason seems to be that 'onContentPrepare' is trigger twice once for ->text and once for ->introtext

https://github.com/joomla/joomla-cms/pull/17175

see also https://github.com/joomla/joomla-cms/issues/17830

I don't say that triggering plugins twice for same text is bad, thus is do not say that PR 17175 is bad

e.g. in this issue we don't have bad side effects

and yes issue #17999 looks same as this one but in that case the double loaded module has some bad side-effects

ghost commented 6 years ago

@ggppdk so i close this Issue as duplicated Report?

ggppdk commented 6 years ago

@franz-wohlkoenig

issue #17999 shows an important side effect of double loading of modules as a result of the double plugin triggering on same text

but then we have another side-effect

i cannot say to close any of them, since people will resubmit them ?

Also the title of this issue is wrong / misleading , and should be fixed !

This issue is about article view that was opened via a link from Article Category List So this issue is about article view !

ghost commented 6 years ago

@ggppdk please have a Look at updated Title.

sandewt commented 6 years ago

Indeed, I have tested using the js code from issue #17999, the title should be about article view. The debugger shows sometimes other results !?


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

sandewt commented 6 years ago

For me is it an issue, because functions will be double loaded, which leads to a fatal error: Cannot redeclare myfunction() / previously declared in...

With the following restriction, it concerns own code! I solve this problem with the following code: _if (!functionexists('myfunction')) { etc.


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

aardgoose commented 6 years ago

Plugins are called twice where the complete text is displayed and no intro text is defined by markers etc.

This is certainly an plugin API change, and in my case breaks a site, and is adding extra processing server side for all pages.

17175 is problematical and changes the plugin API.

chiefguru61 commented 6 years ago

This has broken my site and caused me three days of problems trying to workout why some custom code loaded in articles via jumi and {loadposition xxx} didn't work and other bits did.

Data saved in the session was being processed twice.

I have resolved the issue by moving my custom code out of the article an into another position on the page.


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

regularlabs commented 6 years ago

This is a serious issue. Content plugins are now pretty much all triggered twice. This causes a whole range of issues. The least bad being that pages load slower, as plugins are doing the same thing twice. The worst case scenario is that fatal errors happen because of things being triggered/executed twice that shouldn't be.

When I remove the code added through #17175 the issue is gone. That change should be reverted asap, as it is a B/C change!

laoneo commented 6 years ago

If #17175 get reverted then we need to find a solution how to prepare the intro text. @regularlabs what do you suggest?

regularlabs commented 6 years ago

I guess this is needed for the custom fields? Solve it in the custom fields plugin.

regularlabs commented 6 years ago

The code in #17175 assumes plugins are not already doing stuff to the intro text. But the entire $item is passed. So plugins already have the power and ability to do stuff separately on the $item->introtext, $item->fulltext and $item->text. And many plugins are already doing that.

The onContentPrepare event doesn't expect a single string and do stuff on there. It handles a complete article object.

17175 messes with the values of the article object and just runs onContentPrepare again.

regularlabs commented 6 years ago

Example of what plugins do inside the onContentPrepare:

public function onContentPrepare($context, &$article, &$params)
{
    if (isset($article->title))
    {
        doStuffWithContent($article->title);
    }

    if (isset($article->description))
    {
        doStuffWithContent($article->description);
    }

    if (isset($article->text))
    {
        doStuffWithContent($article->text);

        // don't also do stuff on introtext/fulltext if text is set
        return;
    }

    if (isset($article->introtext))
    {
        doStuffWithContent($article->introtext);
    }

    if (isset($article->fulltext))
    {
        doStuffWithContent($article->fulltext);
    }
}
laoneo commented 6 years ago

Ok, got it, will prepare a pr over the weekend.

regularlabs commented 6 years ago

Ok, great :)

laoneo commented 6 years ago

Please test #18066, it should fix the issue.

joomla-cms-bot commented 6 years ago

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/18016

ghost commented 6 years ago

closed as having Pull Request #18066


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

sandewt commented 6 years ago

Thanks all.