? ?
avatar ggppdk
ggppdk
18 Jul 2015

Joomla 3.4.3

  1. Enable parameter "Add suffix to URL"

  2. Call JHTML::_('behaviour...') inside event onAfterInitialise of any system plugin
    (this triggers JDocument creation)

  3. view any non-HTML view
    e.g.
    http://domain/some-menu-alias.feed

JDocumentHTML is created and displayed instead of JDocumentFeed

avatar ggppdk ggppdk - open - 18 Jul 2015
avatar ggppdk ggppdk - change - 18 Jul 2015
Title
Calling JHTML::_('behaviour...') inside event onAfterIntialize breaks URLs created via parameter "Add suffix to URL" by creating wrong JDocument type
Calling JHTML::_('behaviour...') inside event onAfterInitialize event breaks URLs created via parameter "Add suffix to URL" by creating wrong JDocument type
avatar mbabker
mbabker - comment - 18 Jul 2015

It isn't just JHtml::_('behavior.(...)) but basically anything in the API that ultimately ends up with a call to the JDocument class chain. The application's document object isn't created until after the route has been processed (meaning the first system event it's available in is onAfterDispatch). Being able to do something about that is probably out of scope for the 3.x releases, I would suggest that any plugin code that manipulates the document in onAfterInitialise or onAfterRoute change to a later system event as those two events do not have the document object yet and creating it there can cause unwanted side effects like what you've seen.

avatar ggppdk
ggppdk - comment - 18 Jul 2015

yes, exactly what i was saying, JHtml::_('behavior...) is just an example of forcing JDocument creation

by posting here, i want to raise some awareness about what i found, and if possible to do something for this.

e.g. if possible to move code that handles suffix detection and set 'format' request variable, at an earlier stage, it would prevent such issue. The document creation is using JInput to get 'format', which at onAfterInitialise stage, it is not yet correct.

This will manifest as a bug in various components, including in com_content, e.g. breaking the feeds view, without actually being able to do much for it, and that is the reason of opening this issue

I found the rather popular JCK editors doing this in a system plugin, i have posted an issue report in their website

avatar brianteeman brianteeman - change - 19 Jul 2015
Labels Added: ?
avatar ggppdk
ggppdk - comment - 19 Jul 2015

An alternative to addressing this, could be to add a system message / warning:

$app      = JFactory::getApplication();
$jinput   = $app->input;
$format  = $jinput->get('format', 'html', 'cmd');

if (
    $format && $document->getType() != strtolower($format) &&
    JFactory::getConfig()->get('sef_suffix')
)
{
    $app->enqueueMessage(
        'Document format should be: <b>'.$format.'</b>'
        .' but current document is: <b>'. $document->getType().'</b>'
        .'<br/>Some system plugin may have forced current document type',
    'warning');
}
avatar mbabker
mbabker - comment - 19 Jul 2015

The system message would only confuse the end user, it isn't informative or even helpful IMO. Also, there may be valid use cases where the request format may not match the JDocument object type so that also needs to be considered. Not to sound like I'm trying to "pass the buck", but given how Joomla needs to fully route the request before being able to correctly create the JDocument object, developers need to be discouraged from using the onAfterInitialise and onAfterDispatch methods to do anything that would manipulate a JDocument object (to include calls to a majority of the JHtml helper methods).

avatar Bakual
Bakual - comment - 19 Jul 2015

Closing as it's a bug in the respective plugin doing that and not a bug in core.
Plugins should select the event they fire based on their requirements.

avatar Bakual Bakual - change - 19 Jul 2015
Status New Closed
Closed_Date 0000-00-00 00:00:00 2015-07-19 19:50:28
Closed_By Bakual
avatar Bakual Bakual - close - 19 Jul 2015
avatar Bakual Bakual - close - 19 Jul 2015
avatar Bakual Bakual - close - 19 Jul 2015
avatar mbabker
mbabker - comment - 19 Jul 2015

@Bakual This issue IMO does not fit the definition of "not a bug in core". Granted, the way the system is designed at the moment, it is expected behavior, but it is a highly problematic behavior. I would suggest this remain open with the "Re-evaluate for v4" label (and actually all issues with that label should be open since they are at a minimum considered valid/known issues and have not been dismissed as items that will not be fixed, closing them suggests a "won't fix" status even with a label). As we evaluate the application infrastructure, dependencies such as this one should be looked at and decided if something can be done about it.

avatar ggppdk
ggppdk - comment - 19 Jul 2015

It is not possible to examine many 3rd party system plugins, and warn authors of this

I think that the least thing to do is maybe update documentation here:

https://docs.joomla.org/Plugin/Events/System

Also the benefit of posting here is that now, there is some knowledge about it

avatar ggppdk
ggppdk - comment - 19 Jul 2015

Also the way this behaviour is manifesting, makes it seem like a bug in com_content or any other component, which is misleading.

avatar Bakual Bakual - change - 19 Jul 2015
Labels Added: ?
avatar Bakual
Bakual - comment - 19 Jul 2015

I would suggest this remain open with the "Re-evaluate for v4" label

Added the label.
Back then the idea was to close them to avoid a bit of clutter. It's easy to find them by filtering after the label. We can also always open them again if needed.

I think there are many other potential issues generated by plugins firing when they shouldn't. Most common I encountered being plugins adding inline JavaScript code without checking the document type, breaking also feeds (and other non-HTML output). It's a similar issue to the one found here.
I'm not that convinced if it's something we need to fix. Bad plugins always will be there and breaking sites. It's the responsibility of the plugin developers to choose the correct event.
However if there is a way to prevent such things, it would be great obviously.

Of course the documentation can be improved. It's a wiki, go ahead :+1:

avatar mbabker
mbabker - comment - 19 Jul 2015

Back then the idea was to close them to avoid a bit of clutter. It's easy to find them by filtering after the label.

You and I have different opinions of "avoiding clutter" then :wink: . I consider a known issue tagged for evaluation at a later date as having been acknowledged as at least something worth looking into by the "core team" and should be open, even if it gets no activity for an extended period of time, and a closed issue as one that is resolved or will not be acted on for some reason (hopefully explained in the issue). I personally think closing issues that are known and valid, even if tagged with something that says "come back to this later", at first glance gives the impression that the issue is considered invalid or won't be acted on or if someone's filtering only on open items they could get the impression that the issue is unreported (remember GitHub's search is a bit tricky when switching between their two states).

avatar Bakual
Bakual - comment - 19 Jul 2015

Posted it into the cms-maintainer team. I don't mind either way as long as we handle it consistent. It's only 13 items anyway :smile:

avatar zero-24 zero-24 - change - 24 Aug 2015
Milestone Added:
avatar zero-24 zero-24 - change - 24 Aug 2015
Milestone Added:
avatar ggppdk ggppdk - change - 22 Aug 2016
Title
Calling JHTML::_('behaviour...') inside event onAfterInitialize event breaks URLs created via parameter "Add suffix to URL" by creating wrong JDocument type
Calling JHTML::_('behaviour...') inside event onAfterInitialise event breaks URLs created via parameter "Add suffix to URL" by creating wrong JDocument type
avatar ggppdk ggppdk - edited - 22 Aug 2016
avatar ggppdk ggppdk - edited - 22 Aug 2016

Add a Comment

Login with GitHub to post a comment