Page MenuHomePhabricator

"Internal error" page should expand {{SITENAME}} in 'pagetitle'
Closed, ResolvedPublic

Description

Screenshot – note the title bar

"Internal error" page doesn't expand {{SITENAME}}.


Version: 1.23.0
Severity: minor

Attached:

2013-12-13_14_39_14-Internal_error_-_}_-_Opera.png (395×858 px, 25 KB)

Details

Reference
bz58447

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:20 AM
bzimport set Reference to bz58447.

The most likely cause is 2bcffe8b, but why does it not work, I have no idea.

(In reply to comment #1)

The most likely cause is 2bcffe8b, but why does it not work, I have no idea.

MWException::getPageTitle() was already broken. However, it was not
previously used for DBConnectionErrors.

The problem is that the message is not run through the preprocessor
(e.g. using MessageCache::transform()) before it is inserted into the
HTML.

It might be possible to change Exception::msg() to return a Message object,
now that we have RawMessage, which would make fixing this bug even more
trivial.

Please provide steps to reproduce this as I did verify (and have again, just now) that it works (both general exception, and from db connection errors).

By changing LocalSettings to trigger a connection error, from:

$wgDBtype = 'mysql';
$wgDBserver = 'localhost';
$wgDBpassword = '***';

to:

$wgDBtype = 'mysql';
$wgDBserver = 'localhost';
$wgDBpassword = '***blabla';

By default the message framework should be working fine (just db error), the message blob is used and the following is rendered:

<title>Internal error - alpha.Wiki</title>

When mimicking a broader error (i18n issue and $wgSitename issue), by:

  • Changing MWException::getPageTitle to use inexistent message keys, like 'pagetitle-bla' and 'internalerror-bla'.
  • Changing MWException::getPageTitle to use a different fallback to make it easier to detect (in case was cached or not saved correctly). Like 'Test error' instead of 'Internal error')
  • Removing variables like $wgSitename from LocalSettings.

... then the inline fallback is used and DefaultSettings apply, and the following is rendered:

<title>Test error - MediaWiki</title>

OK, looks like that wasn't a good way to reproduce it because it always uses the fallback in case of a db connection failure, since db connection failure prevents localisation from initialising.

I can reproduce the error when adding:

throw new MWException( 'X');

to a function bound to a parser hook (e.g. 'DoEditSectionLink'), then the following is rendered:

<h1 id="firstHeading">Internal error - {{SITENAME}}</h1>

Makes sense as, indeed, we call wfMessage()->plain().

(In reply to comment #4)

Makes sense as, indeed, we call wfMessage()->plain().

So as a quick fix, changing ->plain() to ->text() might work, assuming
that nothing really needs ->plain().

Change 116080 had a related patch set uploaded by PleaseStand:
Exceptions were directly outputting {{SITENAME}}.

https://gerrit.wikimedia.org/r/116080

(In reply to Kevin Israel (PleaseStand) from comment #2)

(In reply to comment #1)

The most likely cause is 2bcffe8b, but why does it not work, I have no idea.

MWException::getPageTitle() was already broken. However, it was not
previously used for DBConnectionErrors.

It seems that I confused myself. The change looks like it did in fact cause the bug by "[restoring the] sitename as part of error page document title" (for the reason explained in comment 2 and comment 4).

The site name was not already there, and it turns out that MWException::getPageTitle() is not in fact the right place to add it.

  1. The OutputPage code path calls $wgOut->prepareErrorPage().
  2. In turn, OutputPage::setPageTitle() is called.
  3. That method sanitizes the provided HTML for use in the h1 element.
  4. Then it strips any remaining HTML tags and decodes character references.
  5. The resulting string is wrapped in the "pagetitle" message for use in the title element (after re-escaping).

The result of #5 is that when the OutputPage code path is used, the h1 element contains the text "Internal error - {{SITENAME}}" and the title element contains "Internal error - {{SITENAME}} - Wikipedia" (if $wgSitename = "Wikipedia").

It is only the "else" case (OutputPage not used) that is missing the site name, and that is where text() would come in, though it might make sense to keep msg() how it is for now to avoid breaking things (and just replace {{SITENAME}}).

Change 116080 merged by jenkins-bot:
MWException: Expand {{SITENAME}} in pagetitle with Message::text()

https://gerrit.wikimedia.org/r/116080

Not yet fixed for DBConnectionErrors - the originally reported case.

Moving milestone to 1.24. If someone wants to fix it for 1.23, that'd be great, but we can't block release for this.

Change 135443 had a related patch set uploaded by PleaseStand:
DBConnectionError: Expand {{SITENAME}} in pagetitle with Message::text()

https://gerrit.wikimedia.org/r/135443

Change 135443 merged by jenkins-bot:
DBConnectionError: Expand {{SITENAME}} in pagetitle with Message::text()

https://gerrit.wikimedia.org/r/135443