Moodle
  1. Moodle
  2. MDL-36978

Standard Old theme broken if no left blocks configured

    Details

    • Testing Instructions:
      Hide
      1. Login as Admin and select Standardold theme via Theme selector or by URL
      2. In site Home (Frontpage) turn editing ON.
      3. Move all blocks from Left (side-pre) to Right (side-post).
      4. TEST that the page resizes to a side-post-only layout and no-longer triggers the ERROR message as reported.
      Show
      Login as Admin and select Standardold theme via Theme selector or by URL In site Home (Frontpage) turn editing ON. Move all blocks from Left (side-pre) to Right (side-post). TEST that the page resizes to a side-post-only layout and no-longer triggers the ERROR message as reported.
    • Workaround:
      Hide

      Change theme to Standard

      Show
      Change theme to Standard
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-36978_M24
    • Pull Master Branch:
    • Rank:
      46515

      Description

      We have a client that has moved all the standard blocks from the left to right side which meant no blocks on the left side. Testing the upgrade to Moodle 2.3.2 you get this error on logon.

      Replication steps:

      1. Log in as admin
      2. Switch theme to Standard (Legacy)
      3. Navigate to the Front page
      4. Turn editing on
      5. Move all blocks to the right side
      6. Turn editing off

      The following error is displayed.

      Coding error detected, it must be fixed by a programmer: page layout file D:\xampp\htdocs\master_integration/theme/standardold/layout/frontpage.php does not contain the main content placeholder, please include "<?php echo $OUTPUT->main_content() ?>" in theme layout file.
      
      Debug info:
      Error code: codingerror
      Stack trace:
      
          line 746 of \lib\outputrenderers.php: coding_exception thrown
          line ? of unknownfile: call to core_renderer->header()
          line 1416 of \lib\setuplib.php: call to call_user_func_array()
          line 98 of \index.php: call to bootstrap_renderer->__call()
          line 98 of \index.php: call to bootstrap_renderer->header()
      

        Issue Links

          Activity

          Hide
          Mary Evans added a comment -

          The reason you are getting that ERROR is that the theme your client has sounds like it is missing the correct php output for MAIN CONTENT which was changed last year in Moodle 2.1. ALL core theme where updated, so your client's theme must pre-date this change. The correct code is as follows...

          <?php echo $OUTPUT->main_content(); ?>
          

          If you replace the old MAIN_CONTENT_TOKEN code, I think you will find this has nothing to do with moving blocks from left to right, the theme broke because of the fact the old MAIN_CONTENT_TOKEN code was not updated.

          Show
          Mary Evans added a comment - The reason you are getting that ERROR is that the theme your client has sounds like it is missing the correct php output for MAIN CONTENT which was changed last year in Moodle 2.1. ALL core theme where updated, so your client's theme must pre-date this change. The correct code is as follows... <?php echo $OUTPUT->main_content(); ?> If you replace the old MAIN_CONTENT_TOKEN code, I think you will find this has nothing to do with moving blocks from left to right, the theme broke because of the fact the old MAIN_CONTENT_TOKEN code was not updated.
          Hide
          Michael de Raadt added a comment -

          Hi, Tim.

          Could you confirm what Mary suspects? There is no point in us fixing something that isn't a problem.

          Show
          Michael de Raadt added a comment - Hi, Tim. Could you confirm what Mary suspects? There is no point in us fixing something that isn't a problem.
          Hide
          Tim Lock added a comment -

          Hi Michael,

          The core standardold theme looks correct, but if you remove or move all the blocks from the left side of the front page the error will happen even with the correct OUTPUT main content.

          Regards,
          Tim

          Show
          Tim Lock added a comment - Hi Michael, The core standardold theme looks correct, but if you remove or move all the blocks from the left side of the front page the error will happen even with the correct OUTPUT main content. Regards, Tim
          Hide
          Michael de Raadt added a comment -

          Thanks for responding, Tim.

          I was able to replicate that with the Standard (Legacy) theme.

          I have added some replication steps and more error information.

          Mary: Feel free to look at this, otherwise you can assign it to moodle.com and someone else will pick it up.

          Show
          Michael de Raadt added a comment - Thanks for responding, Tim. I was able to replicate that with the Standard (Legacy) theme. I have added some replication steps and more error information. Mary: Feel free to look at this, otherwise you can assign it to moodle.com and someone else will pick it up.
          Hide
          Mary Evans added a comment -

          Well...my first thoughts when I saw the mess that the Frontpage was in I laughed out loud and said...who's the Wally who coded THAT page! LOL

          It must have been like that all the time. LOL
          Sorry but I think it's hilarious.

          I'll fix it ready for next PULL

          Show
          Mary Evans added a comment - Well...my first thoughts when I saw the mess that the Frontpage was in I laughed out loud and said...who's the Wally who coded THAT page! LOL It must have been like that all the time. LOL Sorry but I think it's hilarious. I'll fix it ready for next PULL
          Hide
          Mary Evans added a comment -

          Just attached an amended copy of frontpage.php to keep you going until this is fixed in CORE.
          Cheers
          Mary

          Show
          Mary Evans added a comment - Just attached an amended copy of frontpage.php to keep you going until this is fixed in CORE. Cheers Mary
          Hide
          Mary Evans added a comment - - edited

          @Tim

          I totally missed seeing your commits. I was too busy reading the comments. Why did you not just say that the problem is caused by the incorrect coding of the frontpage.php as demonstrated by your commit messages for master and MOODLE_23_STABLE? It would have saved a lot of time.

          The fact that the main_content column had the <?php if ($hassidepre) { ?> would be reason enough to verify this error, yet you gave the impression the code was correct.

          Also, and perhaps because I was not taking too much notice of this before today, when not only did I see the commit, but also that the way you have done it is not quite right. You should have created a branch called MDL-36978_master based on the master branch and also MDL-36978_M23 based on MOODLE_23_STABLE. I could then have then set them for Integration Review and would be Fixed in the next round of PULLS on Monday. But unfortunately I can't do this with the commits as they are. So to save time I'll do these this evening and get them fixed and set for Integration Review.

          In future, if you do submit a PATCH please say what you have found and how you have fixed the problem. And then ask someone to Peer Review your PATCH. This not only helpful but saves time too.

          We are only too pleased for people to do this.

          And please do accept my apology for not having the brains to see that the fix was on the page all the time, and I did not need to go hunting for it as I did late last night!

          I still find it oddly funny though, that it had not been detected before.

          Cheers & thanks
          Mary

          Show
          Mary Evans added a comment - - edited @Tim I totally missed seeing your commits. I was too busy reading the comments. Why did you not just say that the problem is caused by the incorrect coding of the frontpage.php as demonstrated by your commit messages for master and MOODLE_23_STABLE? It would have saved a lot of time. The fact that the main_content column had the <?php if ($hassidepre) { ?> would be reason enough to verify this error, yet you gave the impression the code was correct. Also, and perhaps because I was not taking too much notice of this before today, when not only did I see the commit, but also that the way you have done it is not quite right. You should have created a branch called MDL-36978 _master based on the master branch and also MDL-36978 _M23 based on MOODLE_23_STABLE. I could then have then set them for Integration Review and would be Fixed in the next round of PULLS on Monday. But unfortunately I can't do this with the commits as they are. So to save time I'll do these this evening and get them fixed and set for Integration Review. In future, if you do submit a PATCH please say what you have found and how you have fixed the problem. And then ask someone to Peer Review your PATCH. This not only helpful but saves time too. We are only too pleased for people to do this. And please do accept my apology for not having the brains to see that the fix was on the page all the time, and I did not need to go hunting for it as I did late last night! I still find it oddly funny though, that it had not been detected before. Cheers & thanks Mary
          Hide
          Mary Evans added a comment - - edited

          I have just found MDL-23226 where the original error was first discovered in Moodle 2.1. Unfortunately only standardold/layout/general.php was fixed, that's why frontpage.php was the odd one out in this.

          At least I can tie up the lose ends now.

          Show
          Mary Evans added a comment - - edited I have just found MDL-23226 where the original error was first discovered in Moodle 2.1. Unfortunately only standardold/layout/general.php was fixed, that's why frontpage.php was the odd one out in this. At least I can tie up the lose ends now.
          Hide
          Mary Evans added a comment -

          @Tim
          Although the code looks OK, the commits need to be on separate branches prefixed with MDL-36978 as well as labelled and based on their respective stable_backlog Moodle branches, including master.

          Because time is short I'll get them done now and submit for integration ASAP.

          Show
          Mary Evans added a comment - @Tim Although the code looks OK, the commits need to be on separate branches prefixed with MDL-36978 as well as labelled and based on their respective stable_backlog Moodle branches, including master. Because time is short I'll get them done now and submit for integration ASAP.
          Hide
          Mary Evans added a comment -

          @Sam

          I have added you as a Peer Reviewer for this as you were involved in a similar fix in MDL-23226 for much the same thing as this bug fix.

          Show
          Mary Evans added a comment - @Sam I have added you as a Peer Reviewer for this as you were involved in a similar fix in MDL-23226 for much the same thing as this bug fix.
          Hide
          Sam Hemelryk added a comment -

          Thanks Mary, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Mary, this has been integrated now
          Hide
          Ankit Agarwal added a comment -

          works as described!

          Show
          Ankit Agarwal added a comment - works as described!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: