Can you give an example of when

Because JPATH_COMPONENT is not always $this->_basePath

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9123.}

I agree this looks wrong, especially since the check is for JPATH_COMPONENT and 4 lines later it does exactly the same as in this line.

can you do a PR to fix this issue? Then it can properly be tested/reviewed. If you can, also give some testing instructions as this greatly helps to get that fixed.

See https://docs.joomla.org/Using_the_Github_UI_to_Make_Pull_Requests for how to create a PR.

Actually, I think the wrong path is in the elseif clause. It should be

elseif (is_dir($this->_basePath . '/view'))

When you have custom base path for your view, I don't think it is logic to check for the view folder inside the component

Full code is:

        // Set the default template search path
        if (array_key_exists('template_path', $config))
        {
            // User-defined dirs
            $this->_setPath('template', $config['template_path']);
        }
        elseif (is_dir(JPATH_COMPONENT . '/view'))
        {
            $this->_setPath('template', $this->_basePath . '/view/' . $this->getName() . '/tmpl');
        }
        else
        {
            $this->_setPath('template', $this->_basePath . '/views/' . $this->getName() . '/tmpl');
        }

If the elseif check is wrong, the also the code in the last else is wrong.

We don't use custom base_path anywhere in Joomla core(all the views are inside view or views folder inside component folder), so It is hard to tell which code is wrong

However, as I understand, the purpose of the code is to support view located in both views and view folder. As we define base path, I think it is more logical to check it base on $this->_basePath instead of hardcode to JPATH_COMPONENT. I also could not see what's wrong with the code in the last else.

@joomdonation Patch it. As $this->_basePath is defined just before this, it's the right call.

@brianteeman com_media has a case where $this->_basePath is not always JPATH_COMPONENT. The admin entry file overrides the base path to always be JPATH_COMPONENT_ADMINISTRATOR since frontend com_media uses are just proxying into the admin component in full.

I also could not see what's wrong with the code in the last else.

@joomdonation My fault, didn't notice the views check. I thought it's the same line.
You're doing the PR?

@mbabker Thanks

@Bakual Yes, I will do this litle PR. Just don't know how to give testing instructions :D

I think as long as the views still work, it's quite ok.
And maybe also check the instances mentioned by Michael.

OK. PR created. We can close this issue now. If testing instructions is asked, I will try to provide instructions (have never used media manager from frontend), for now, I hope a code review from maintainer could be enough.

@joomdonation just change this line:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_media/controller.php#L67

have never used media manager from frontend

@joomdonation The media form field uses com_media. From the frontend layout overrides (because some people do style the modal for their template) should still work.

Closed please see #9128

Sorry that i dont checked before for answer.
I don't know if old code can break something or not. But this was simply illogic on read.
I think that in all case this had some "else" for nothing, so i think all your comment clarify it.
If JPATH_COMPONENT is always $this->_basePath then why control for this ?
Because at next step no dir is controlled.

I think you understand why i have trouble with this. Perhaps it worked, but something, here, is/was strange.