I confirm the issue. This is a regression.

When layout==='pagebreak'

• then the base_path is the backend to load the backend model which does not have the hits() method https://github.com/joomla/joomla-cms/blob/0e22713228dba29f2fbd3c045a27572883e0cd5a/components/com_content/controller.php#L32-L36

// Article frontpage Editor pagebreak proxying:
if ($this->input->get('view') === 'article' && $this->input->get('layout') === 'pagebreak')
{
    $config['base_path'] = JPATH_COMPONENT_ADMINISTRATOR;
}

but the problem is not loading the backend model,
neither the recent change should be reverted :

• as it fixed hits not being counted when page is page is not cacheable 0e22713

instead add an extra check if

• method hits() exists and (also ?) if article model is the frontend model

https://github.com/joomla/joomla-cms/blob/0e22713228dba29f2fbd3c045a27572883e0cd5a/components/com_content/controller.php#L109-L111

@alikon
So i suggest using:

    // Get/Create the model
    if ( $model = $this->getModel($vName) )
    {
        if (method_exists($model, 'hit'))
            $model->hit();
        }
    }

[EDIT]
i have corrected function name 'hits' which should be 'hit'

Another solution I tested and works fine is:

        if ($vName == 'article')
        {
            // Get/Create the model
            if ($model = $this->getModel($vName) && JFactory::getApplication()->input->get('layout') !== 'pagebreak')
            {
                $model->hit();
            }
        }

Please, @alikon , test my solution and I will make PR.

Simpler:

        if ($vName == 'article')
        {
            // Get/Create the model
            if ($model = $this->getModel($vName) && $this->input->get('layout') !== 'pagebreak')
            {
                $model->hit();
            }
        }

Making PR now.

@infograf768
no better avoid it,
because it will break at any time that controler or other code decides to use backend model

the check if the model has hits() method must be there

I let you do then.

Sorry folks unable to help till evening...
On 6 Apr 2016 11:16 am, "infograf768" notifications@github.com wrote:

I let you do then. [image: ]

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9748 (comment)

@infograf768
i cannot make anything till late evening
can you please make a PR for this: (so that it is done faster)

    // Get/Create the model
    if ( $model = $this->getModel($vName) )
    {
        if (method_exists($model, 'hit'))  // check for backend model, do not move the check above, it will fail
        {
            $model->hit();
        }
    }

[EDIT]
i have corrected function name 'hits' which should be 'hit'

I guess you mean "hit" instead of "hits" which does not exist...
I.e.
if (method_exists($model, 'hit')) // do not move the check above, it will fail

yes, sorry, function name is 'hit'

I let you do, folks. It can wait tonight as I guess we are not going to release a 3.5.2 because of this regression.
It would be good to add this though in the docs.

To be on the safe side, maybe one could do

        if ($vName == 'article')
        {
            // Get/Create the model
            if ($model = $this->getModel($vName))
            {
                if (method_exists($model, 'hit')
                    && $this->input->get('layout') !== 'pagebreak')
                {
                    $model->hit();
                }
            }
        }

although it looks like checking the layout is not really necessary.

yes that is even better
in the case that a future change makes pagebreak use frontend model, which would cause a hit to be counted every time the pagebreak popup is opened

Based on this template as a model: https://docs.joomla.org/J3.x:Login/Logout_Redirections_broken_after_upgrade_to_Joomla_3.4.6
Thank you to report this issue/fix in this category: https://docs.joomla.org/Category:Version_3.5.1_FAQ

@Sandra97 done at: https://docs.joomla.org/J3.x:Backend/Page_Break_PHP_error_after_upgrade_to_Joomla_3.5.1 please make sure I have not broken it ;)

@zero-24, perfect! Thank you!

@ggppdk @infograf768 If we use is_callable() instead of method_exists() , wouldn't that be better ?
Meanwhile can I make a PR ?

Seriously, the solution is check if a method exists!? That's frankly as much crap as the logic was in 3.5.0. If another model adds a hit method and that model is used in the display method what assurance do you have that it being called is the intended behavior when something is displayed? Even worse, since hit doesn't have a defined interface anywhere, what if a new hit method is added with parameters?

Checking on the layout is a terrible choice too (and it's already bad this is happening in the content.php file AND the controller's constructor as a means to decide ACL or what classes are included).

if article model is the frontend model

You can't do that in Joomla. ContentModelArticle is the model's class name in both site and admin. You can use Reflection to check file paths, but honestly that's not a good choice either. It shouldn't be an option since you can overload component MVC classes with plugins. We already get enough people yelling about B/C breaks when plugins stop being able to overload classes because they get pushed into the autoloader.

Honestly, we (all of us) built ourselves into a hole. Why are we using one display task to deal with the logic of a dozen view combinations when we extend and override other action tasks (add, edit, save) to fix context?

+1

On 4/6/2016 07:45, Michael Babker wrote:

Seriously, the solution is check if a method exists!? That's frankly
as much crap as the logic was in 3.5.0. If another model adds a hit
method and that model is used in the display method what assurance do
you have that it being called is the intended behavior when something
is displayed? Even worse, since hit doesn't have a defined interface
anywhere, what if a new hit method is added with parameters?

Checking on the layout is a terrible choice too (and it's already bad
this is happening in the content.php file AND the controller's
constructor as a means to decide ACL or what classes are included).

if article model is the frontend model

You can't do that in Joomla. |ContentModelArticle| is the model's
class name in both site and admin. You can use Reflection to check
file paths, but honestly that's not a good choice either. It shouldn't
be an option since you can overload component MVC classes with
plugins. We already get enough people yelling about B/C breaks when
plugins stop being able to overload classes because they get pushed
into the autoloader.

Honestly, we (all of us) built ourselves into a hole. Why are we using
one display task to deal with the logic of a dozen view combinations
when we extend and override other action tasks (add, edit, save) to
fix context?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#9748 (comment)

@mbabker
what solution do you propose?

I can't say I have one that isn't one of the existing ways of checking without a B/C break but honestly the method_exists() check seems like it's the most prone to error in the future and the suggestion of checking the model is also pretty weak. At least the layout check matches up to one specific use case.

why not with an event trigger like this dirty one ??
in the joomla content plugin

        public function onContentAfterDisplay($context, &$row, &$params, $page=0)
    {
        if ($context == 'com_content.article')
        {
            $table = JTable::getInstance('Content', 'JTable');
            $table->hit($row->id);
        }
        return null;
    }
`

Personally, I don't think core behaviors like this should be in a plugin. And if you're going to make hit counting a plugin driven function I'd say it should be more abstract than supporting a single extension.

....feedback is warmly wellcommed ...

p.s.
as usual my quick&dirty fix

See #9768

@brianteeman do you mean #9766 ?

yes - oops