Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-32684

Setting Theme-Priority with $CFG->themeorder does not work correctly

          Details

          • Testing Instructions:
            Hide
            1. In config php set: $CFG->themeorder = array('course', 'category', 'session', 'user', 'site');
            2. Go To Admin > Appearance > Themes > Theme Settings
            3. Enable user themes and course themes
            4. Go to your user profile and edit your theme to be anonomoly
            5. Verify theme is is changed to anomoly on all pages
            6. Create a course and set the theme to splash
            7. Verify theme is changed to splash
            8. Change your config.php to:
              $CFG->themeorder = array('user', 'course', 'category', 'session', 'site');
            9. Verify that the theme visible in the course is now anomoly not splash.
            Show
            In config php set: $CFG->themeorder = array('course', 'category', 'session', 'user', 'site'); Go To Admin > Appearance > Themes > Theme Settings Enable user themes and course themes Go to your user profile and edit your theme to be anonomoly Verify theme is is changed to anomoly on all pages Create a course and set the theme to splash Verify theme is changed to splash Change your config.php to: $CFG->themeorder = array('user', 'course', 'category', 'session', 'site'); Verify that the theme visible in the course is now anomoly not splash.
          • Workaround:
            Hide

            None

            Show
            None
          • Difficulty:
            Easy
          • Affected Branches:
            MOODLE_22_STABLE
          • Fixed Branches:
            MOODLE_21_STABLE, MOODLE_22_STABLE
          • Pull from Repository:
          • Pull Master Branch:
          • Pull Master Diff URL:

            Description

            When using $CFG->themeorder = array('session', 'course', 'category', 'user', 'site'); Moodle will take the site-Theme before the course-Theme because of missing "break" in the switch-case Block of the function resolve_theme() in lib/pagelib.php.

              Gliffy Diagrams

                Attachments

                  Issue Links

                  is a regression caused by

                  New Feature - A new Moodle feature which has yet to be developed. MDL-19077 Implement the theme engines proposal

                  • Critical - Crashes server, loss of data, severe memory leak
                  • Closed

                    Activity

                    Ascending order - Click to sort in descending order
                    Hide
                    Permalink
                    lazydaisy Mary Evans added a comment -

                    Thank you for reporting this issue and for the patch, Andreas, however this is not a 'Themes' problem, although it is related, it actually comes under Administration as the fix would need to be done in CORE files and not Themes. So, in view of this, I have just changed the component from 'Themes' to 'Administration' so one of the Moodle Developers can look at this.

                    Thanks again
                    Mary

                    Show
                    lazydaisy Mary Evans added a comment - Thank you for reporting this issue and for the patch, Andreas, however this is not a 'Themes' problem, although it is related, it actually comes under Administration as the fix would need to be done in CORE files and not Themes. So, in view of this, I have just changed the component from 'Themes' to 'Administration' so one of the Moodle Developers can look at this. Thanks again Mary
                    Hide
                    Permalink
                    salvetore Michael de Raadt added a comment -

                    Thanks for spotting that and providing a patch.

                    Show
                    salvetore Michael de Raadt added a comment - Thanks for spotting that and providing a patch.
                    Hide
                    Permalink
                    salvetore Michael de Raadt added a comment -

                    Hi, Tim.

                    I thought you might be interested in looking at this potential fix as you were involved in the original code.

                    Show
                    salvetore Michael de Raadt added a comment - Hi, Tim. I thought you might be interested in looking at this potential fix as you were involved in the original code.
                    Hide
                    Permalink
                    timhunt Tim Hunt added a comment -

                    Oh, I see. Yes, good catch Andreas. I can't believe this bug has been there for nearly two years, and no-one has noticed. This fix should be turned into a git commit, and submitted for integration.

                    (It took me a while to work out why the break; statements were necessary, given that there are all the return statements there, but they are necessary.)

                    Show
                    timhunt Tim Hunt added a comment - Oh, I see. Yes, good catch Andreas. I can't believe this bug has been there for nearly two years, and no-one has noticed. This fix should be turned into a git commit, and submitted for integration. (It took me a while to work out why the break; statements were necessary, given that there are all the return statements there, but they are necessary.)
                    Hide
                    Permalink
                    lazydaisy Mary Evans added a comment -

                    Thanks Tim, if it's OK I can do this.

                    Show
                    lazydaisy Mary Evans added a comment - Thanks Tim, if it's OK I can do this.
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Added more verbose testing instructions.

                    Show
                    poltawski Dan Poltawski added a comment - Added more verbose testing instructions.
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Integrated, thanks

                    Show
                    poltawski Dan Poltawski added a comment - Integrated, thanks
                    Hide
                    Permalink
                    rajeshtaneja Rajesh Taneja added a comment -

                    Works Great
                    Thanks for fixing this Mary and Andreas.

                    Show
                    rajeshtaneja Rajesh Taneja added a comment - Works Great Thanks for fixing this Mary and Andreas.
                    Hide
                    Permalink
                    stronk7 Eloy Lafuente (stronk7) added a comment -

                    This is now part of Moodle and a few millions people around the globe will be using it soon. Isn't that awesome?

                    Many, many thanks and don't forget http://youtu.be/4N7dPaP5Z8U

                    Closing, ciao

                    Show
                    stronk7 Eloy Lafuente (stronk7) added a comment - This is now part of Moodle and a few millions people around the globe will be using it soon. Isn't that awesome? Many, many thanks and don't forget http://youtu.be/4N7dPaP5Z8U Closing, ciao

                      People

                      • Votes:
                        1 Vote for this issue
                        Watchers:
                        2 Start watching this issue

                        Dates

                        • Created:
                          Updated:
                          Resolved:
                          Fix Release Date:
                          14/May/12