? Success

User tests: Successful: Unsuccessful:

avatar astridx
astridx
14 Jan 2019

Pull Request for Issue # #23510 . (Second try :))

Summary of Changes

I deleted the !$prefix condition from two if statements in the libraries/src/MVC/Factory/MVCFactory.php class. I was looking really carefully. But I can not find a place where$prefix = 'site'is correct if strpos($config['base_path'], '/administrator/') is true = if $config['base_path'] is set and contains the word '/administrator/'.

The condition !$prefix prevents that $prefix = 'Administrator'; can be set. $prefix is set in the line

$prefix = $this->app->getName();

to $prefix = 'Site;.

With this patch, no error will appear any more.

Testing Instructions / Expected result / Actual result

See #23510

Documentation Changes Required

No

What I noticed

What still confuses me is the fact that not all menu items can be selected.

I seldom work with the front end editing. That's why I did not notice that a publisher in xtd-modal does not see all menu items, which he can click in the font end. But: That was the same with Joomla 3x. I just checked that. Or is there something missing here?

avatar astridx astridx - open - 14 Jan 2019
avatar astridx astridx - change - 14 Jan 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 14 Jan 2019
Category Libraries
avatar astridx astridx - change - 14 Jan 2019
Title
fix for XTD plugins broken in frontend
[4.0] fix for XTD plugins broken in frontend
avatar astridx astridx - change - 14 Jan 2019
Title
fix for XTD plugins broken in frontend
[4.0] fix for XTD plugins broken in frontend
avatar astridx astridx - edited - 14 Jan 2019
avatar joomdonation
joomdonation - comment - 14 Jan 2019

I haven't had time to read and understand current MVC implementation but I think MVCFactory should not contains magic code like that. Maybe a better solution for this problem would be override createView and createModel method directly in DisplayController of com_content frontend, something like below code:

	protected function createModel($name, $prefix = '', $config = array())
	{
		if (!$prefix && $this->input->get('view') === 'articles' && $this->input->get('layout') === 'modal')
		{
			$prefix = 'Administrator';
		}

		return parent::createModel($name, $prefix, $config);
	}

	
	protected function createView($name, $prefix = '', $type = '', $config = array())
	{
		if (!$prefix && $this->input->get('view') === 'articles' && $this->input->get('layout') === 'modal')
		{
			$prefix = 'Administrator';
		}

		return parent::createView($name, $prefix, $type, $config);
	}

The same logic would be applied for view = article and layout = pagebreak

avatar astridx
astridx - comment - 16 Jan 2019

@joomdonation Thank you for your comment.

I looked at it again. I am far away from properly understanding the MVCFactory class. But I think that the functionality belongs here. The function is not only needed in com_content. Each component may use a backend view in the frontend. That is why I think it is general and it belongs to the parent class

But: This special case must be specified in the child class. If we agree that in this case the variable $config['base_path'] is set, it would run properly. That's how it was in Joomla 3. But changes in Joomla 4 have caused that the variable $prefix is set and that is why the line I changed in this PR did not work any more.

I think this is what @mbabker means here: #23530 (comment) with

which is a regression from 3.x where the components were smart enough to proxy frontend requests to the backend code

I see no reason why $prefix needs to be checked in the if statement I changed.

avatar joomdonation
joomdonation - comment - 16 Jan 2019

There must be a reason for that check. I am unsure but from my quick look, with your change might break the case when we try to create a frontend model (in this case, we would pass 'Site' as the value for $prefix parameter)

Also, as I mentioned, we are having too much magic code to determine the $prefix base on base_path config value. That's why I think override createModel and createView method when it's needed would be better.

For this, we need someone with better experience to make decision.

avatar SharkyKZ
SharkyKZ - comment - 28 Jun 2019

@laoneo can you take a look at this?

avatar SharkyKZ
SharkyKZ - comment - 28 Jun 2019

It seems that the prefix must always be set in Joomla\CMS\MVC\Controller\BaseController::getView(). Otherwise Joomla\CMS\MVC\Controller\BaseController::$views ends up with prefix-less views, which causes conflicts.

avatar astridx
astridx - comment - 30 Jun 2019

Close because of other PR #25367

avatar astridx astridx - change - 30 Jun 2019
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2019-06-30 09:04:51
Closed_By astridx
avatar astridx astridx - close - 30 Jun 2019
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 30 Jun 2019

Thanks for this PR @astridx

Add a Comment

Login with GitHub to post a comment