Moodle
  1. Moodle
  2. MDL-30330

Some actions can unexpectedly initialise page layout

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.1.2
    • Fix Version/s: DEV backlog
    • Component/s: Libraries
    • Labels:
    • Affected Branches:
      MOODLE_21_STABLE
    • Rank:
      32684

      Description

      We had a couple of cases recently of confusing bugs in our (non-core) code related to the way themes and page layout work.

      In one case there were certain situations where the module ended up calling format_text before it had initialised the page. This basically causes everything to break. You do get a developer warning at this point that tells you that you haven't initialised page context, so this is fair enough.

      But we have also had a similar problem where a page started displaying in the wrong page layout unexpectedly (it otherwise worked OK because in that case we had already set up the context) because we changed something that caused a call to format_text before setting the page layout.

      Here are some independent suggestions for improvement:

      1) Currently, when it initialises the theme it also fixes the page layout. This could be changed to a two-step process so that you can initialise the theme and the rest of the page object, but without fixing the actual page layout. The page layout should only be fixed when $OUTPUT->header is called. (See the example code in MDL-30329.) The reason this is important is that there are many unpredictable things you can do - not just directly calling format_text, but calling some function which happens to call some function which happens to call format_text, etc - which end up fixing the page layout unexpectedly.

      2) If you call set_pagelayout after the page layout has already been fixed, this should print a developer debug warning (note: I would prefer to make it throw a coding_exception but I suspect there is existing broken code in this area).

      3) Could add to format_text phpdoc to mention that you must not call it before setting page context and otherwise initialising the page.

        Issue Links

          Activity

          Hide
          Davo Smith added a comment -

          This issue just hit me due to an add-on activity using $OUTPUT->pix_url as part of the settings page. modedit.php does not call $PAGE->set_pagelayout('admin') until after the activity has inserted its settings, so I got an obscure javascript error (and a missing TinyMCE editor) when the initialised regions from the 'base' layout (side-pre, side-post) did not match the regions in the 'admin' layout output by $OUTPUT->header (side-pre).

          Is there anyone looking into a fix for this?

          Show
          Davo Smith added a comment - This issue just hit me due to an add-on activity using $OUTPUT->pix_url as part of the settings page. modedit.php does not call $PAGE->set_pagelayout('admin') until after the activity has inserted its settings, so I got an obscure javascript error (and a missing TinyMCE editor) when the initialised regions from the 'base' layout (side-pre, side-post) did not match the regions in the 'admin' layout output by $OUTPUT->header (side-pre). Is there anyone looking into a fix for this?
          Hide
          Sam Marshall added a comment -

          Davo: I'm not looking into this at present. Unfortunately I guess this is one of those things where, once you figure it out you can fix your individual problem, and it would be too much work to fix the general one, so the general 'this behaviour sucks' stays broken.

          Show
          Sam Marshall added a comment - Davo: I'm not looking into this at present. Unfortunately I guess this is one of those things where, once you figure it out you can fix your individual problem, and it would be too much work to fix the general one, so the general 'this behaviour sucks' stays broken.

            People

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

              Dates

              • Created:
                Updated: