Moodle
  1. Moodle
  2. MDL-40213

Clean theme layout "secure.php" missing settings.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.5, 2.6
    • Fix Version/s: 2.5.4, 2.6.1, 2.7, FRONTEND
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. Select the 'Clean' theme.
      2. Turn on developer level debugging.
      3. Create a course.
      4. Add a quiz and in 'Extra restrictions on attempts' -> 'Browser security' set to 'Full screen pop-up with some JavaScript security'. See http://docs.moodle.org/26/en/Quiz_settings#Extra_restrictions_on_attempts for more information.
      5. Add one question.
      6. Go to the quiz and click on the 'Attempt quiz now'.
      7. Observe that the error occurs and that the course name is not displayed at the top.
      8. Apply the patch.
      9. Refresh the page and observe that the error no longer occurs and the course name is displayed at the top.
      Show
      Select the 'Clean' theme. Turn on developer level debugging. Create a course. Add a quiz and in 'Extra restrictions on attempts' -> 'Browser security' set to 'Full screen pop-up with some JavaScript security'. See http://docs.moodle.org/26/en/Quiz_settings#Extra_restrictions_on_attempts for more information. Add one question. Go to the quiz and click on the 'Attempt quiz now'. Observe that the error occurs and that the course name is not displayed at the top. Apply the patch. Refresh the page and observe that the error no longer occurs and the course name is displayed at the top.
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE, MOODLE_27_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-40213_master

      Description

      I noticed a bug in the secure page layout for the clean theme. It is missing this code from the head:

      // Get the HTML for the settings bits.
      $html = theme_clean_get_html_for_settings($OUTPUT, $PAGE);

      Without this - it should get an undefined variable warning later in the page when it writes the page header.

      With developer level debugging on, the following errors are reported:

      Notice: Undefined variable: html in F:\moodledev\moodlegjb\theme\clean\layout\secure.php on line 51

      Notice: Trying to get property of non-object in F:\moodledev\moodlegjb\theme\clean\layout\secure.php on line 51

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Gareth J Barnard added a comment -

            Just bumped into this whilst looking at the layout files in M2.6 - I could do the fix but do not have a secure site upon which to test. Mary Evans thoughts?

            Show
            Gareth J Barnard added a comment - Just bumped into this whilst looking at the layout files in M2.6 - I could do the fix but do not have a secure site upon which to test. Mary Evans thoughts?
            Hide
            Mary Evans added a comment - - edited

            I am not sure if that layout should have the settings as this is meant to be a secure page where test take place, usually using quizes. It's not so much a secure site but rather a secure browser that you need to download that stops student surfing when in a test situation. As far as I recall Sam added this layout to Bootstrapbase, so I suspect he left that off deliberately.

            Anyway all that code does is allows the logo in the header. If this layout needs a header then add it in.

            Best person to ask about the browser is Tim Hunt

            Show
            Mary Evans added a comment - - edited I am not sure if that layout should have the settings as this is meant to be a secure page where test take place, usually using quizes. It's not so much a secure site but rather a secure browser that you need to download that stops student surfing when in a test situation. As far as I recall Sam added this layout to Bootstrapbase, so I suspect he left that off deliberately. Anyway all that code does is allows the logo in the header. If this layout needs a header then add it in. Best person to ask about the browser is Tim Hunt
            Hide
            Mary Evans added a comment -

            I've assigned this to you Gareth

            Show
            Mary Evans added a comment - I've assigned this to you Gareth
            Hide
            Mary Evans added a comment -

            MDL-31365 for more info on the origin of the 'Secure' layout.

            Show
            Mary Evans added a comment - MDL-31365 for more info on the origin of the 'Secure' layout.
            Hide
            Mary Evans added a comment - - edited

            If you check this out Gareth you will also find that the secure layout in bootstrapbase/config.php is missing vital options...
             

            'options' => array('nofooter'=>true, 'nonavbar'=>true,
            'nocustommenu'=>true, 'nologinlinks'=>true,
            'nocourseheaderfooter'=>true),

            Which to my way of thinking is more important!

            Show
            Mary Evans added a comment - - edited If you check this out Gareth you will also find that the secure layout in bootstrapbase/config.php is missing vital options...   'options' => array('nofooter'=>true, 'nonavbar'=>true, 'nocustommenu'=>true, 'nologinlinks'=>true, 'nocourseheaderfooter'=>true), Which to my way of thinking is more important!
            Hide
            Tim Hunt added a comment -

            The 'secure' layout is used by the quiz when you turn on the option Browser security: Pop-up window with some JavaScript. No special browsers required to test this.

            Show
            Tim Hunt added a comment - The 'secure' layout is used by the quiz when you turn on the option Browser security: Pop-up window with some JavaScript. No special browsers required to test this.
            Hide
            Mary Evans added a comment -

            In that case Gareth, going on how Afterburner treats this is to add the missing line of code back. Jobs a good un!

            Show
            Mary Evans added a comment - In that case Gareth, going on how Afterburner treats this is to add the missing line of code back. Jobs a good un!
            Hide
            Gareth J Barnard added a comment -

            Hi Mary Evans,

            Indeed - just going to follow Tim Hunt's instructions (thanks Tim).

            Cheers,

            Gareth

            Show
            Gareth J Barnard added a comment - Hi Mary Evans , Indeed - just going to follow Tim Hunt 's instructions (thanks Tim). Cheers, Gareth
            Hide
            Rossiani Wijaya added a comment -

            Hi Gareth,

            Thank you for fixing this issue.

            Patch look great.

            This should be integrated to 2.5, 2.6 and master.

            Submitting for integration review.

            Show
            Rossiani Wijaya added a comment - Hi Gareth, Thank you for fixing this issue. Patch look great. This should be integrated to 2.5, 2.6 and master. Submitting for integration review.
            Hide
            Marina Glancy added a comment -

            Thanks Gareth, integrated in 2.5, 2.6 and master.
            I cherry-picked commit to 2.6 branch but can you please create a separate branch next time. Thanks again.

            Show
            Marina Glancy added a comment - Thanks Gareth, integrated in 2.5, 2.6 and master. I cherry-picked commit to 2.6 branch but can you please create a separate branch next time. Thanks again.
            Hide
            Gareth J Barnard added a comment - - edited

            Thanks Marina Glancy, I thought M2.6 and master branches were linked after the release of M2.6 until M2.6.1 as was with M2.5 and master until M2.5.1. Must be mistaken and will do.

            Show
            Gareth J Barnard added a comment - - edited Thanks Marina Glancy , I thought M2.6 and master branches were linked after the release of M2.6 until M2.6.1 as was with M2.5 and master until M2.5.1. Must be mistaken and will do.
            Hide
            Marina Glancy added a comment -

            The on-sync period when 2.6 was kept [almost] exactly the same as 2.7 was only 2 weeks. And even during this period we integrate to 2.6 and master branches separately.

            Show
            Marina Glancy added a comment - The on-sync period when 2.6 was kept [almost] exactly the same as 2.7 was only 2 weeks. And even during this period we integrate to 2.6 and master branches separately.
            Hide
            Michael de Raadt added a comment -

            Test result: Success!

            Tested in 2.5, 2.6 and master. Tested as admin and student.

            No errors were shown in Integration version.

            Thanks for your efforts.

            Show
            Michael de Raadt added a comment - Test result: Success! Tested in 2.5, 2.6 and master. Tested as admin and student. No errors were shown in Integration version. Thanks for your efforts.
            Hide
            Sam Hemelryk added a comment -

            Thanks for the code, its now upstream!

            Heres a fun trick to try in the spirit of Friday the 13th.
            I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

            Show
            Sam Hemelryk added a comment - Thanks for the code, its now upstream! Heres a fun trick to try in the spirit of Friday the 13th. I hear if you stand in front a mirror, alone, in the dark, and say "Oracle" three times Petr Skoka will appear in the mirror and you'll see him deleting the Oracle driver from Moodle.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: