Graph must not use synchronous `window.mw` check (doesn't work asynchronous)
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Krinkle
	Aug 5 2015, 1:18 AM

Description

Graph currently uses document.write. More-over, it hardcodes if(window.mw) (instead of using ResourceLoader::makeLoaderConditionalScript) which means it is broken in current master because this condition changed.

Graph.body.php:

	$imgTagHtml::rawElement( 'img', array( 'class' => 'mw-wiki-graph-img', .. );
	..
if ( $imgTag ) {
	$encImg = FormatJson::encode( $imgTag, false, FormatJson::UTF8_OK );
	$result .= Html::inlineScript( 'if(!window.mw){document.write(' . $encImg . ');}' ) .
			Html::rawElement( 'noscript', array(), $imgTag );

If you need inline javascript that runs only when jQuery/ResourceLoader initialise, use ResourceLoader::makeInlineScript (which is a wrapper around Html::inlineScript and ResourceLoader::makeLoaderConditionalScript).

However in this case it actually does the opposite. It checks for absence rather than presence of window.mw. And also uses <noscript>. This no longer works in current MediaWiki master because mediawiki.js loads asynchronously.

Instead of this javascript and noscript code, use a plain image with a class and hide it in CSS. E.g. .client-js .mw-graph-something { display: none; }.

Details

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	Improve fallback rendering logic	mediawiki/extensions/Graph	wmf/1.26wmf17	+21 -16
	Improve fallback rendering logic	mediawiki/extensions/Graph	master	+21 -16

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		ori	T108197 Blockers for 1.26wmf18 (released on 2015-08-11)
		Resolved		Yurik	T107976 Graph must not use synchronous `window.mw` check (doesn't work asynchronous)

Event Timeline

Krinkle created this task.Aug 5 2015, 1:18 AM

Krinkle assigned this task to Yurik.

Krinkle raised the priority of this task from to Unbreak Now!.

Krinkle updated the task description. (Show Details)

Krinkle added a project: MediaWiki-extensions-Graph.

Krinkle added subscribers: Krinkle, Yurik.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 5 2015, 1:18 AM

Krinkle lowered the priority of this task from Unbreak Now! to High.Aug 5 2015, 1:44 AM

Krinkle mentioned this in T105924: Graph extension should exclude scripts tags so they will not be parsed by TextExtracts.

Krinkle updated the task description. (Show Details)

Krinkle set Security to None.

@Krinkle, what would you recommend to achieve the following behaviour:

For non-javascript browsers, insert <img> tag
For browsers with old javascript (e.g. IE7), insert the <img> tag (same as non-js)
For modern browsers, insert the <div> tag

In T107976#1510915, @Yurik wrote:

@Krinkle, what would you recommend to achieve the following behaviour:

For non-javascript browsers, insert <img> tag

For browsers with old javascript (e.g. IE7), insert the <img> tag (same as non-js)

For modern browsers, insert the <div> tag

Use javascript (delivered as ResourceLoader module) to render the <div> tag. This is naturally not applied to non-js and old browsers.
Use a plain <img> element for the fallback that is hidden by default and made visible with CSS using its classname as descendent of .client-nojs. Using e.g. .client-js .mw-graph-something { display: none; } (in an addModuleStyles top module).