From b7c4c8717f964d1890d185ec3e6e9481fcb734e4 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 18 Jul 2016 12:52:10 -0700 Subject: [PATCH] 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 --- RELEASE-NOTES-1.28 | 1 + docs/hooks.txt | 14 +---- includes/OutputPage.php | 11 +++- includes/parser/Parser.php | 70 +++++++++------------- includes/parser/ParserOutput.php | 42 +++++++------ includes/resourceloader/ResourceLoader.php | 2 +- 6 files changed, 65 insertions(+), 75 deletions(-) diff --git a/RELEASE-NOTES-1.28 b/RELEASE-NOTES-1.28 index 339cbfa380..608bc56da4 100644 --- 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 e1b397472d..57240c98f9 100644 --- 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 f6119802e2..c8a385a510 100644 --- 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 a765450f4e..e57ec8ec49 100644 --- 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\n"; + } - // Sanitize for comment. Note '‐' in the replacement is U+2010, - // which looks much like the problematic '-'. - $limitReport = str_replace( [ '-', '&' ], [ '‐', '&' ], $limitReport ); - $text .= "\n\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"; + $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 73d4e69008..f0528125b0 100644 --- 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 6596112c48..59f646228e 100644 --- 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 ); } -- 2.20.1