Magento-lts: Admin theme changed by default config value

[AlterWeb]

Preconditions (*)

1. OpenMage 19.4.7

Steps to reproduce (*)

1. Change config value of design/theme/template and design/theme/skin to a random theme under the default config

Expected result (*)

1. All stores use this value for there skin unless specified different
2. The admin theme should be openmage

Actual result (*)

1. All stores use this value for there skin unless specified different
2. The default Magento admin theme is used because it can not find the random theme (unless this is "openmage" of course)

[aterjung]

Thank you for writing this bug report! There is something wrong in the theme switcher code. I will fix this as soon as possible. As long as this is fixed and included in the next release, you could just move the skin overwrite to the website level.

[AlterWeb]

@aterjung Thank you for the quick response. I think this issue is not new but we never noticed it because the (old) default theme it is falling back to was the expected behavior. If I remember correctly from debugging it, the code responsable for this behavior was not edited. You even may say that it's a feature, but if it is than it is a confusing one ;-). Thank you for noticing that workaround, we already did do exactly that for now. So no big problem for now but it makes the configuration a bit more confusing.

[aterjung]

@AlterWeb where is the code you identified as source for this error? I am a little bit confused at the moment, my dev-environment where the theme switcher was developed does not show this behavior! But all of my customers live systems do have this error.

[AlterWeb]

I don't have access to my workstation at the moment (typing this on my phone) but if I remember correctly it was in the class Mage_Adminhtml_Controller_Action. Around line 163 there is a foreach loop setting those values. I can check this tomorrow.

[AlterWeb]

@aterjung I just checked it and this is indeed where those values are set. So this code:

foreach (array('layout', 'template', 'skin', 'locale') as $type) {
    if ($value = (string)Mage::getConfig()->getNode("stores/admin/design/theme/{$type}")) {
        Mage::getDesign()->setTheme($type, $value);
    }
}

[kiatng]

I think it's worth considering making the new _openmage_ theme as a separate package, which will resolve this issue and answer issue #1164. I made the following test, which requires renaming the current default/default directory to base/default like in frontend. The complete changes required are:

Make a new openmage package

1. Move contents from app/design/adminhtml/default/openmage to new directory app/design/adminhtml/openmage/default.
2. Move contents from skin/adminhtml/default/openmage to new directory skin/adminhtml/openmage/default.

Change default/default to base/default

1. Change package directory name from app/design/adminhtml/default/ to app/design/adminhtml/base/.
2. Change skin directory name from skin/adminhtml/default/ to skin/adminhtml/base/.

Add theme.xml in openmage

Add new file: app/design/adminhtml/openmage/default/etc/theme.xml

<?xml version="1.0"?>
<!--
/**
 *
 * @category    design
 * @package     openmage_default
 */
-->
<theme>
    <parent />
</theme>

This file is copied from the frontend packages. See, for example rwd's theme.xml.

Setting the package and theme in config.xml

In app\code\core\Mage\Adminhtml\etc\config.xml, the following can be removed in entirety:

    <stores>
        <admin>
            <design>
                <package>
                    <name>default</name>
                </package>
                <theme>
                    <default>default</default>
                    <openmage>openmage</openmage>
                </theme>
            </design>
        </admin>
    </stores>

After I removed the above, the backend is the old admin theme.

To use _openmage_ package, we can add the following in any config.xml file:

    <stores>
        <admin>
            <design>
                <package>
                    <name>openmage</name>
                </package>
                <theme>
                    <default>default</default>
                </theme>
            </design>
        </admin>
    </stores>

The above can be set in a switcher.

More testing required

With the above changes, we should have infiite theme fallback. It should work but I have not tested it thoroughly.

[aterjung]

@kiatng thank you for this. This would be a solution for #1164 at least. I will prepare a fork for easy testing.

I am not shure if this can fix the strange behavior of the Mage_Adminhtml_Controller_Action`s preDispatch method. Probably i miss the point that is handled here:

        Mage::getDesign()
            ->setArea($this->_currentArea)
            ->setPackageName((string)Mage::getConfig()->getNode('stores/admin/design/package/name'))
            ->setTheme((string)$theme);
        foreach (array('layout', 'template', 'skin', 'locale') as $type) {
            if ($value = (string)Mage::getConfig()->getNode("stores/admin/design/theme/{$type}")) {
                Mage::getDesign()->setTheme($type, $value);
            }
        }

For me it looks like it handles the selected theme layout/template/skin/locale settings for the backend. getConfig()->getNode should get the values form xml config. Indeed the returned values are the ones for the frontend (defined in core_config_data) if there are no xml definitions. I wasn't aware of this fallback until now. This happens as well for the package name. If you remove

 <stores>
        <admin>
            <design>
                <package>
                    <name>default</name>
                </package>

the package you define in the backend will be used for the adminhtml area!

Setting default values in the config.xml fixes this but breaks the theme switch that is implemented now:

   <stores>
        <admin>
            <design>
                <package>
                    <name>default</name>
                </package>
                <theme>
                    <default>default</default>
                    <openmage>openmage</openmage>
                    <layout>openmage</layout>
                    <template>openmage</template>
                    <skin>openmage</skin>
                    <locale>openmage</locale>
                </theme>
            </design>
        </admin>
    </stores>

Thus, i think migration to a backend theme package let this problem disappear without resolving it. If i get it right, this is a problem of bad design back to the stating days of magento. :-) The backend theme uses the config of storeId 0 which is the "default config" everybody uses to put the fronted theme in. To set it fixed to "adminhtml" this value is overridden by xml config - but only for the package - and "default". If you add a "skin"-Overwrite (for the frontend) in the backend it is not found in adminhtml and therefore ignored falling back to "default".

Do we really need the layout, skin, template and locale overwrite for backend themes? Probably we should drop this foreach an fix the values there?

[kiatng]

Continue with my comment, I tested custom theme rewrite based on it.

Custom Theme Rewrite in Custom Package

As an example, I want to rewrite this file app/design/adminhtml/base/default/template/customer/tab/view.phtml.

Copy and paste the file to new package called ccs, the full path is then app/design/adminhtml/ccs/default/template/customer/tab/view.phtml.

Then I added this: <h1>Theme Rewrite in Package ccs</h1> to the copied file.

Add theme.xml

Add a new file app/design/adminhtml/ccs/default/etc/theme.xml

<?xml version="1.0"?>
<!--
/**
 *
 * @category    design
 * @package     ccs_default
 */
-->
<theme>
    <parent>openmage/default</parent>
</theme>

Note: if the file is missing, it will fallback to the package base. But the rewrite still works:

Set the Custom Package

In any config.xml:

    <stores>
        <admin>
            <design>
                <package>
                    <name>ccs</name>
                </package>
                <theme>
                    <default>default</default>
                </theme>
            </design>
        </admin>
    </stores>

Voila:

Issues

1. As noted, this breaks the current theme switcher in backend configuration. It seems the proper way to do this is by having something similar for frontend, where the user can configure the package, theme, layout, skin, etc.
2. The change require moving the entire directory app/design/adminhtml/default/default to app/design/adminhtml/base/default, BC? Instead of move, we can copy, which create bloatware?

[aterjung]

@kiatng have you read my comment about the way the adminhtml package is forced in the Controller?

Mage::getConfig()->getNode('stores/admin/design/package/name') will return this line from core_config_data if no package is defined for store/admin in any config.xml:

That is because the admin area is considered as store "0". The Problem is that the "default" config for the frontend themes is also saved with scope_id 0. Normaly there will always be a config.xml setting the adminhtml package to a different value. If you put a overwrite for "template" for example in the backend config in "default" scope, lets say "mytemp", then this value form db will be set for the backend theme as well - but it is intended for frontend. That is becase no config.xml setting is made for the template path. If you use the default theme, that will not harm, cause there is no folder "mytemp" in the adminhtml folder which let the value gracefully fall back to "default". But using the openmage theme, the fallback to "default" is not what we want.

The change to an own package you suggest will heal this symptom of a broken design at first, because the theme would be called "default" again. But everybody wo builds his own custom theme by adding a theme to the openmage package would ran into this problem again.

From my point of view, we have to fix this "combined" config path for frontend and backend theme to get this fixed. If the openmage theme should be a package or a theme is a different discussion then.

[aterjung]

@kiatng I tested this with the same results. I build up theme packages just like you showed and created a custom package which I set as design package from config.xml file!

Going on from this point, my PR #1229 reverts the changes to Mage_Adminhtml_Controller_Action::preDispatch method for the legacy theme switcher and introduces a new option to overwrite admin theme from backend. This is probably not the final version, but it should work without problems for your example and let you chose which design package you want right from the backend. It would be great if you could confirm that this works for you.