Move NewPP limit report HTML comments to JS variables

* Instead of having messy code to create a hidden HTML
  comment of English strings at the bottom of the page,
  expose the structured data of the parse information
  to JS so tools can use it.
* Make makeConfigSetScript() use pretty output so these
  variables are also easy to read in "view source".
* Remove ParserLimitReportFormat hook, since the data
  is not formatted to HTML anymore.

Bug: T110763
Change-Id: I2783c46c6d80f828f9ecf5e71fc8f35910454582

diff --git a/RELEASE-NOTES-1.28 b/RELEASE-NOTES-1.28
index 339cbfa..608bc56 100644 (file)
--- a/RELEASE-NOTES-1.28
+++ b/RELEASE-NOTES-1.28
@@ -84,6 +84,7 @@ changes to languages because of Phabricator reports.
   MediaWiki\Linker\LinkRenderer. In addition, the LinkBegin and LinkEnd hooks
   were replaced by HtmlPageLinkRendererBegin and HtmlPageLinkRendererEnd
   respectively. See docs/hooks.txt for the specific changes needed for those hooks.
+* The 'ParserLimitReportFormat' hook was removed.
 
 == Compatibility ==
 

diff --git a/docs/hooks.txt b/docs/hooks.txt
index e1b3974..57240c9 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -2360,24 +2360,12 @@ cache or return false to not use it.
 &$parser: Parser object
 &$varCache: variable cache (array)
 
-'ParserLimitReport': DEPRECATED! Use ParserLimitReportPrepare and
-ParserLimitReportFormat instead.
+'ParserLimitReport': DEPRECATED! Use ParserLimitReportPrepare instead.
 Called at the end of Parser:parse() when the parser will
 include comments about size of the text parsed.
 $parser: Parser object
 &$limitReport: text that will be included (without comment tags)
 
-'ParserLimitReportFormat': Called for each row in the parser limit report that
-needs formatting. If nothing handles this hook, the default is to use "$key" to
-get the label, and "$key-value" or "$key-value-text"/"$key-value-html" to
-format the value.
-$key: Key for the limit report item (string)
-&$value: Value of the limit report item
-&$report: String onto which to append the data
-$isHTML: If true, $report is an HTML table with two columns; if false, it's
-  text intended for display in a monospaced font.
-$localize: If false, $report should be output in English.
-
 'ParserLimitReportPrepare': Called at the end of Parser:parse() when the parser
 will include comments about size of the text parsed. Hooks should use
 $output->setLimitReportData() to populate data. Functions for this hook should

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index f611980..c8a385a 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -290,6 +290,9 @@ class OutputPage extends ContextSource {
         */
        private $copyrightUrl;
 
+       /** @var array Profiling data */
+       private $limitReportData = [];
+
        /**
         * Constructor for OutputPage. This should not be called directly.
         * Instead a new RequestContext should be created and it will implicitly create
@@ -1754,11 +1757,14 @@ class OutputPage extends ContextSource {
                        }
                }
 
-               // enable OOUI if requested via ParserOutput
+               // Enable OOUI if requested via ParserOutput
                if ( $parserOutput->getEnableOOUI() ) {
                        $this->enableOOUI();
                }
 
+               // Include profiling data
+               $this->limitReportData = $parserOutput->getLimitReportData();
+
                // Link flags are ignored for now, but may in the future be
                // used to mark individual language links.
                $linkFlags = [];
@@ -3075,7 +3081,8 @@ class OutputPage extends ContextSource {
         * @return string
         */
        function getBottomScripts() {
-               return $this->getScriptsForBottomQueue();
+               return $this->getScriptsForBottomQueue() .
+                       Skin::makeVariablesScript( [ 'wgPageParseReport' => $this->limitReportData ] );
        }
 
        /**

diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index a765450..e57ec8e 100644 (file)
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -501,60 +501,46 @@ class Parser {
                                [ $this->mHighestExpansionDepth, $this->mOptions->getMaxPPExpandDepth() ]
                        );
                        $this->mOutput->setLimitReportData( 'limitreport-expensivefunctioncount',
-                               [ $this->mExpensiveFunctionCount, $this->mOptions->getExpensiveParserFunctionLimit() ]
+                               [ $this->mExpensiveFunctionCount,
+                                       $this->mOptions->getExpensiveParserFunctionLimit() ]
                        );
                        Hooks::run( 'ParserLimitReportPrepare', [ $this, $this->mOutput ] );
 
-                       $limitReport = "NewPP limit report\n";
-                       if ( $wgShowHostnames ) {
-                               $limitReport .= 'Parsed by ' . wfHostname() . "\n";
-                       }
-                       $limitReport .= 'Cached time: ' . $this->mOutput->getCacheTime() . "\n";
-                       $limitReport .= 'Cache expiry: ' . $this->mOutput->getCacheExpiry() . "\n";
-                       $limitReport .= 'Dynamic content: ' .
-                               ( $this->mOutput->hasDynamicContent() ? 'true' : 'false' ) .
-                               "\n";
-
-                       foreach ( $this->mOutput->getLimitReportData() as $key => $value ) {
-                               if ( Hooks::run( 'ParserLimitReportFormat',
-                                       [ $key, &$value, &$limitReport, false, false ]
-                               ) ) {
-                                       $keyMsg = wfMessage( $key )->inLanguage( 'en' )->useDatabase( false );
-                                       $valueMsg = wfMessage( [ "$key-value-text", "$key-value" ] )
-                                               ->inLanguage( 'en' )->useDatabase( false );
-                                       if ( !$valueMsg->exists() ) {
-                                               $valueMsg = new RawMessage( '$1' );
-                                       }
-                                       if ( !$keyMsg->isDisabled() && !$valueMsg->isDisabled() ) {
-                                               $valueMsg->params( $value );
-                                               $limitReport .= "{$keyMsg->text()}: {$valueMsg->text()}\n";
-                                       }
-                               }
-                       }
-                       // Since we're not really outputting HTML, decode the entities and
-                       // then re-encode the things that need hiding inside HTML comments.
-                       $limitReport = htmlspecialchars_decode( $limitReport );
+                       $limitReport = '';
                        Hooks::run( 'ParserLimitReport', [ $this, &$limitReport ] );
+                       if ( $limitReport != '' ) {
+                               // Sanitize for comment. Note '‐' in the replacement is U+2010,
+                               // which looks much like the problematic '-'.
+                               $limitReport = str_replace( [ '-', '&' ], [ '‐', '&' ], $limitReport );
+                               $text .= "\n<!-- \nNewPP limit report\n$limitReport-->\n";
+                       }
 
-                       // Sanitize for comment. Note '‐' in the replacement is U+2010,
-                       // which looks much like the problematic '-'.
-                       $limitReport = str_replace( [ '-', '&' ], [ '‐', '&amp;' ], $limitReport );
-                       $text .= "\n<!-- \n$limitReport-->\n";
-
-                       // Add on template profiling data
+                       // Add on template profiling data in human/machine readable way
                        $dataByFunc = $this->mProfiler->getFunctionStats();
                        uasort( $dataByFunc, function ( $a, $b ) {
                                return $a['real'] < $b['real']; // descending order
                        } );
-                       $profileReport = "Transclusion expansion time report (%,ms,calls,template)\n";
+                       $profileReport = [];
                        foreach ( array_slice( $dataByFunc, 0, 10 ) as $item ) {
-                               $profileReport .= sprintf( "%6.2f%% %8.3f %6d - %s\n",
-                                       $item['%real'], $item['real'], $item['calls'],
-                                       htmlspecialchars( $item['name'] ) );
+                               $profileReport[] = sprintf( "%6.2f%% %8.3f %6d %s",
+                                       $item['%real'], $item['real'], $item['calls'], $item['name'] );
                        }
-                       $text .= "\n<!-- \n$profileReport-->\n";
+                       $this->mOutput->setLimitReportData( 'limitreport-timingprofile', $profileReport );
 
-                       if ( $this->mGeneratedPPNodeCount > $this->mOptions->getMaxGeneratedPPNodeCount() / 10 ) {
+                       // Add other cache related metadata
+                       if ( $wgShowHostnames ) {
+                               $this->mOutput->setLimitReportData( 'cachereport-origin', wfHostname() );
+                       }
+                       $this->mOutput->setLimitReportData( 'cachereport-timestamp',
+                               $this->mOutput->getCacheTime() );
+                       $this->mOutput->setLimitReportData( 'cachereport-ttl',
+                               $this->mOutput->getCacheExpiry() );
+                       $this->mOutput->setLimitReportData( 'cachereport-transientcontent',
+                               $this->mOutput->hasDynamicContent() );
+
+                       if ( $this->mGeneratedPPNodeCount
+                               > $this->mOptions->getMaxGeneratedPPNodeCount() / 10
+                       ) {
                                wfDebugLog( 'generated-pp-node-count', $this->mGeneratedPPNodeCount . ' ' .
                                        $this->mTitle->getPrefixedDBkey() );
                        }

diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php
index 73d4e69..f052812 100644 (file)
--- a/includes/parser/ParserOutput.php
+++ b/includes/parser/ParserOutput.php
@@ -188,9 +188,7 @@ class ParserOutput extends CacheTime {
         */
        private $mExtensionData = [];
 
-       /**
-        * @var array $mLimitReportData Parser limit report data.
-        */
+       /** @var array $mLimitReportData Parser limit report data. */
        private $mLimitReportData = [];
 
        /**
@@ -981,24 +979,34 @@ class ParserOutput extends CacheTime {
        /**
         * Sets parser limit report data for a key
         *
-        * The key is used as the prefix for various messages used for formatting:
-        *  - $key: The label for the field in the limit report
-        *  - $key-value-text: Message used to format the value in the "NewPP limit
-        *      report" HTML comment. If missing, uses $key-format.
-        *  - $key-value-html: Message used to format the value in the preview
-        *      limit report table. If missing, uses $key-format.
-        *  - $key-value: Message used to format the value. If missing, uses "$1".
-        *
-        * Note that all values are interpreted as wikitext, and so should be
-        * encoded with htmlspecialchars() as necessary, but should avoid complex
-        * HTML for sanity of display in the "NewPP limit report" comment.
+        * If $value consist of a list of two floats, it will be interpreted as
+        * (actual value, maximum allowed value). The presence of a "-" in $key will cause
+        * the first part of the key to be interpreted as a namespace.
         *
         * @since 1.22
-        * @param string $key Message key
-        * @param mixed $value Appropriate for Message::params()
+        * @param string $key Data key
+        * @param mixed $value Data value One of (float, string, bool, JSON serializable array)
         */
        public function setLimitReportData( $key, $value ) {
-               $this->mLimitReportData[$key] = $value;
+               if ( is_array( $value ) ) {
+                       if ( array_keys( $value ) === [ 0, 1 ]
+                               && is_numeric( $value[0] )
+                               && is_numeric( $value[1] )
+                       ) {
+                               $data = [ 'value' => $value[0], 'limit' => $value[1] ];
+                       } else {
+                               $data = $value;
+                       }
+               } else {
+                       $data = $value;
+               }
+
+               if ( strpos( $key, '-' ) ) {
+                       list( $ns, $name ) = explode( '-', $key, 2 );
+                       $this->mLimitReportData[$ns][$name] = $data;
+               } else {
+                       $this->mLimitReportData[$key] = $data;
+               }
        }
 
        /**

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 6596112..59f6462 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1394,7 +1394,7 @@ MESSAGE;
                return Xml::encodeJsCall(
                        'mw.config.set',
                        [ $configuration ],
-                       ResourceLoader::inDebugMode()
+                       true // readable
                );
        }