Exception: Clean up html document for error pages
authorTimo Tijhof <krinklemail@gmail.com>
Thu, 19 Sep 2013 16:22:51 +0000 (18:22 +0200)
committerBartosz Dziewoński <matma.rex@gmail.com>
Wed, 30 Oct 2013 21:52:49 +0000 (21:52 +0000)
Interface:
* Restore sitename as part of error page document title
  (follows-up 4ca9805). Moved to general Exception class as it
  is useful in general, this effectively changes the title from
  "This wiki has a problem" to "Internal error - Wikipedia".
* Added basic <style> to have it use a sans-serif font and a bit
  padding to offset it from the edge of the window.
* Output stacktrace in <pre> as-is (with linebreaks) instead of
  in a <p> with <br/>.

Clean up:
* Removed spurious "<!-- SiteSearch Google -->" comment.
* Change sitesearch radio button to not need the id/for
  attributes but simply nest them. Visual rendering and browser
  behaviour is identical.
* Renamed $text to $html since that is what it is (follows-up
  4ca9805 which reused that variable for html, it used to contain
  text).
* Remove odd linebreak from "dberr-problems" message, it was
  being output as-is in the html (html escaped, of course) and
  line breaks have no meaning inside an <h1> tag. Using a simple
  space instead. Visual rendering is identical.

Coding style:
* Using <!DOCTYPE html> instead of <!doctype html> for consistency
  with other documents we output.
* Switched a few inline #-style comments to use // instead.
* Switched a few strings from double quotes to single quotes in
  areas close to the changed code.

Change-Id: I33232d871200cbd23501c9a6c5f8a178931e135e

includes/Exception.php
includes/db/DatabaseError.php
languages/messages/MessagesEn.php

index a91f865..46402f9 100644 (file)
@@ -164,7 +164,8 @@ class MWException extends Exception {
         * @return string
         */
        function getPageTitle() {
-               return $this->msg( 'internalerror', "Internal error" );
+               global $wgSitename;
+               return $this->msg( 'pagetitle', "$1 - $wgSitename", $this->msg( 'internalerror', 'Internal error' ) );
        }
 
        /**
@@ -209,13 +210,14 @@ class MWException extends Exception {
 
                        $wgOut->output();
                } else {
-                       header( "Content-Type: text/html; charset=utf-8" );
-                       echo "<!doctype html>\n" .
+                       header( 'Content-Type: text/html; charset=utf-8' );
+                       echo "<!DOCTYPE html>\n" .
                                '<html><head>' .
                                '<title>' . htmlspecialchars( $this->getPageTitle() ) . '</title>' .
+                               '<style>body { font-family: sans-serif; margin: 0; padding: 0.5em 2em; }</style>' .
                                "</head><body>\n";
 
-                       $hookResult = $this->runHooks( get_class( $this ) . "Raw" );
+                       $hookResult = $this->runHooks( get_class( $this ) . 'Raw' );
                        if ( $hookResult ) {
                                echo $hookResult;
                        } else {
@@ -242,8 +244,8 @@ class MWException extends Exception {
                } elseif ( self::isCommandLine() ) {
                        MWExceptionHandler::printError( $this->getText() );
                } else {
-                       header( "HTTP/1.1 500 MediaWiki exception" );
-                       header( "Status: 500 MediaWiki exception", true );
+                       header( 'HTTP/1.1 500 MediaWiki exception' );
+                       header( 'Status: 500 MediaWiki exception', true );
                        header( "Content-Type: $wgMimeType; charset=utf-8", true );
 
                        $this->reportHTML();
index da391d7..937bea0 100644 (file)
@@ -66,8 +66,7 @@ class DBError extends MWException {
                $s = $this->getHTMLContent();
 
                if ( $wgShowDBErrorBacktrace ) {
-                       $s .= '<p>Backtrace:</p><p>' .
-                               nl2br( htmlspecialchars( $this->getTraceAsString() ) ) . '</p>';
+                       $s .= '<p>Backtrace:</p><pre>' . htmlspecialchars( $this->getTraceAsString() ) . '</pre>';
                }
 
                return $s;
@@ -137,24 +136,17 @@ class DBConnectionError extends DBError {
         * @return bool
         */
        function getLogMessage() {
-               # Don't send to the exception log
+               // Don't send to the exception log
                return false;
        }
 
-       /**
-        * @return string
-        */
-       function getPageTitle() {
-               return $this->msg( 'dberr-header', 'This wiki has a problem' );
-       }
-
        /**
         * @return string
         */
        function getHTML() {
                global $wgShowDBErrorBacktrace, $wgShowHostnames, $wgShowSQLErrors;
 
-               $sorry = htmlspecialchars( $this->msg( 'dberr-problems', "Sorry!\nThis site is experiencing technical difficulties." ) );
+               $sorry = htmlspecialchars( $this->msg( 'dberr-problems', 'Sorry! This site is experiencing technical difficulties.' ) );
                $again = htmlspecialchars( $this->msg( 'dberr-again', 'Try waiting a few minutes and reloading.' ) );
 
                if ( $wgShowHostnames || $wgShowSQLErrors ) {
@@ -169,17 +161,16 @@ class DBConnectionError extends DBError {
                # No database access
                MessageCache::singleton()->disable();
 
-               $text = "<h1>$sorry</h1><p>$again</p><p><small>$info</small></p>";
+               $html = "<h1>$sorry</h1><p>$again</p><p><small>$info</small></p>";
 
                if ( $wgShowDBErrorBacktrace ) {
-                       $text .= '<p>Backtrace:</p><p>' .
-                               nl2br( htmlspecialchars( $this->getTraceAsString() ) ) . '</p>';
+                       $html .= '<p>Backtrace:</p><pre>' . htmlspecialchars( $this->getTraceAsString() ) . '</pre>';
                }
 
-               $text .= '<hr />';
-               $text .= $this->searchForm();
+               $html .= '<hr />';
+               $html .= $this->searchForm();
 
-               return $text;
+               return $html;
        }
 
        protected function getTextContent() {
@@ -195,21 +186,21 @@ class DBConnectionError extends DBError {
        public function reportHTML() {
                global $wgUseFileCache;
 
-               # Check whether we can serve a file-cached copy of the page with the error underneath
+               // Check whether we can serve a file-cached copy of the page with the error underneath
                if ( $wgUseFileCache ) {
                        try {
                                $cache = $this->fileCachedPage();
-                               # Cached version on file system?
+                               // Cached version on file system?
                                if ( $cache !== null ) {
-                                       # Hack: extend the body for error messages
+                                       // Hack: extend the body for error messages
                                        $cache = str_replace( array( '</html>', '</body>' ), '', $cache );
-                                       # Add cache notice...
-                                       $cache .= '<div style="color:red;font-size:150%;font-weight:bold;">' .
+                                       // Add cache notice...
+                                       $cache .= '<div style="border:1px solid #ffd0d0;padding:1em;">' .
                                                htmlspecialchars( $this->msg( 'dberr-cachederror',
-                                                       'This is a cached copy of the requested page, and may not be up to date. ' ) ) .
+                                                       'This is a cached copy of the requested page, and may not be up to date.' ) ) .
                                                '</div>';
 
-                                       # Output cached page with notices on bottom and re-close body
+                                       // Output cached page with notices on bottom and re-close body
                                        echo "{$cache}<hr />{$this->getHTML()}</body></html>";
                                        return;
                                }
@@ -218,7 +209,7 @@ class DBConnectionError extends DBError {
                        }
                }
 
-               # We can't, cough and die in the usual fashion
+               // We can't, cough and die in the usual fashion
                parent::reportHTML();
        }
 
@@ -239,8 +230,8 @@ class DBConnectionError extends DBError {
 
                $trygoogle = <<<EOT
 <div style="margin: 1.5em">$usegoogle<br />
-<small>$outofdate</small></div>
-<!-- SiteSearch Google -->
+<small>$outofdate</small>
+</div>
 <form method="get" action="//www.google.com/search" id="googlesearch">
        <input type="hidden" name="domains" value="$server" />
        <input type="hidden" name="num" value="50" />
@@ -249,12 +240,11 @@ class DBConnectionError extends DBError {
 
        <input type="text" name="q" size="31" maxlength="255" value="$search" />
        <input type="submit" name="btnG" value="$googlesearch" />
-  <div>
-       <input type="radio" name="sitesearch" id="gwiki" value="$server" checked="checked" /><label for="gwiki">$sitename</label>
-       <input type="radio" name="sitesearch" id="gWWW" value="" /><label for="gWWW">WWW</label>
-  </div>
+       <p>
+               <label><input type="radio" name="sitesearch" value="$server" checked="checked" />$sitename</label>
+               <label><input type="radio" name="sitesearch" value="" />WWW</label>
+       </p>
 </form>
-<!-- SiteSearch Google -->
 EOT;
                return $trygoogle;
        }
@@ -266,15 +256,17 @@ EOT;
                global $wgTitle, $wgOut, $wgRequest;
 
                if ( $wgOut->isDisabled() ) {
-                       return ''; // Done already?
+                       // Done already?
+                       return '';
                }
 
-               if ( $wgTitle ) { // use $wgTitle if we managed to set it
+               if ( $wgTitle ) {
+                       // use $wgTitle if we managed to set it
                        $t = $wgTitle->getPrefixedDBkey();
                } else {
-                       # Fallback to the raw title URL param. We can't use the Title
-                       # class is it may hit the interwiki table and give a DB error.
-                       # We may get a cache miss due to not sanitizing the title though.
+                       // Fallback to the raw title URL param. We can't use the Title
+                       // class is it may hit the interwiki table and give a DB error.
+                       // We may get a cache miss due to not sanitizing the title though.
                        $t = str_replace( ' ', '_', $wgRequest->getVal( 'title' ) );
                        if ( $t == '' ) { // fallback to main page
                                $t = Title::newFromText(
index 6d68d36..fb8457f 100644 (file)
@@ -4956,8 +4956,7 @@ You should have received [{{SERVER}}{{SCRIPTPATH}}/COPYING a copy of the GNU Gen
 
 # Database error messages
 'dberr-header'      => 'This wiki has a problem',
-'dberr-problems'    => 'Sorry!
-This site is experiencing technical difficulties.',
+'dberr-problems'    => 'Sorry! This site is experiencing technical difficulties.',
 'dberr-again'       => 'Try waiting a few minutes and reloading.',
 'dberr-info'        => '(Cannot contact the database server: $1)',
 'dberr-info-hidden' => '(Cannot contact the database server)',