Odd question about $PAGE->set_context()

Odd question about $PAGE->set_context()

by Mark Johnson -
Number of replies: 5
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

If you don't call require_login() on a Moodle 2 page, you get a warning saying that the page's context hasn't been set.  I always understood that part of the intention of this was to make sure that you do call require_login() for security, or that you ensure you set the context manually if you don't need the page behind login[1].

However, it was just pointed out to me that even though an error message is displayed, so is the page's content.  Should this not display as a coding error that stops the page being displyed until it's fixed, especially if there are security implications?

[1] http://moodle.org/mod/forum/discuss.php?d=164908

Average of ratings: Useful (1)
In reply to Mark Johnson

Re: Odd question about $PAGE->set_context()

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

Some things won't work properly if $PAGE->context is not right, so that is why there is a warning if you fail to set it.

The point is not really to force you to call require_login. You should know to do that anyway. If that is a side-effect of the warning, then good. It helps developers get their code right. And, for that reason, a warning really ought to be sufficient. You would not release code that outputs a warning, would you?

Average of ratings: -
In reply to Tim Hunt

Re: Odd question about $PAGE->set_context()

by Mark Johnson -
Picture of Core developers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

You're right, I wouldn't publish code that contains a warning. However I'm not sure that the warning message itself makes the severity of the problem clear enough, or really explains how to solve it. When I first saw the warning I thought I needed to do $PAGE->context = $context which didn't work.

Given the potential severity of the problem (at best, bits of the page not working. At worst, unauthenticated users accessing the page) should the message, if not treated as a coding error, not give a bit more detail about the problem?  Something like "The page's context is not set.  You may have forgotten to call require_login() or $PAGE->set_context().  The page may not display correctly as a result."

Average of ratings: -
In reply to Mark Johnson

Re: Odd question about $PAGE->set_context()

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers

Would you like to

  1. Create a patch to improve the error message.
  2. Create a tracker issue.
  3. Attach the patch to the issue.
  4. Ping me

Then I will review it. Better than creating a patch would be to make your chage as a commit on github.

Average of ratings: -