r89198 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r89197‎ | r89198 | r89199 >
Date:02:12, 31 May 2011
Author:erik
Status:ok (Comments)
Tags:
Comment:
Suppress AFT from all print outputs. I am adding a @media rule, but also
suppressing rendering of the widget in the &printable version (which does
not respect the @media setting when rendered to the screen). I tried just
using the noprint class, but it does not appear to be present in the older
skins.
Modified paths:
  • /trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.startup.js (modified) (history)
  • /trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.css (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.css
——@@ -8,6 +8,12 @@
99 margin-top: 1em;
1010 }
1111
 12+@media print {
 13+ .articleFeedback {
 14+ display:none;
 15+ }
 16+}
 17+
1218 .articleFeedback-panel {
1319 background-color: #f9f9f9;
1420 border: 1px solid #cccccc;
Index: trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.startup.js
——@@ -15,6 +15,8 @@
1616 && mw.util.getParamValue( 'oldid' ) == null
1717 // Not viewing a redirect
1818 && mw.util.getParamValue( 'redirect' ) != 'no'
 19+ // Not viewing the printable version
 20+ && mw.util.getParamValue( 'printable' ) != 'yes'
1921 ) {
2022 // Assign a tracking bucket using options from wgArticleFeedbackTracking
2123 mw.user.bucket(

Follow-up revisions

RevisionCommit summaryAuthorDate
r893551.17wmf1: MFT r88146, r88151, r89198, r89274catrope18:54, 2 June 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r89192don't load AFT on printable versionerik22:47, 30 May 2011
r89193meh, should be done via CSSerik23:02, 30 May 2011

Comments

#Comment by 😂 (talk | contribs)   23:55, 1 June 2011

Looks good.

#Comment by He7d3r (talk | contribs)   19:22, 2 June 2011

I noticed a small inconsistency:

I'm not sure how MediaWiki interprets the "printable" parameter but, empirically, from the examples above I suppose it is not checking for the exact value "yes", since any non-empty/non-zero value seems to make the page "printable".

Does anyone knows exactly how this parameter should be interpreted?

#Comment by Krinkle (talk | contribs)   11:28, 5 June 2011

Nowhere in MediaWiki-core are links to printable=(anything), except to printable=yes.

It's undocumented behaviour that should not be supported in anyway.

#Comment by Krinkle (talk | contribs)   11:38, 5 June 2011

Since I couldn't reproduce the effects on en.wikipedia locally I dug into this a little deeper.


./includes/Wiki.php:

This is what happends in trunk (view markup): 

	public function performRequest() {
		global $wgServer, $wgUsePathInfo;

		wfProfileIn( __METHOD__ );

		if ( $this->context->request->getVal( 'printable' ) === 'yes' ) {
			$this->context->output->setPrintable();
		}


And here's what happends in 1.17wmf1 (view markup): 

	function checkInitialQueries( $title, $action ) {
		global $wgOut, $wgRequest, $wgContLang;
		if( $wgRequest->getVal( 'printable' ) === 'yes' ) {
			$wgOut->setPrintable();
		}

Aside from being a different method, the result is the same. The changemaker is in OutputPage.php where the resourceloader-urls are contructed.

./includes/OutputPage.php: Here's trunk (view markup) 

		// Propagate printable and handheld parameters if present
		if ( $this->isPrintable() ) {
			$baseQuery['printable'] = 1;
		}

And here's 1.17 view markup) 

		// Propagate printable and handheld parameters if present
		 if ( $wgRequest->getBool( 'printable' ) ) {
			$baseQuery['printable'] = 1;
		}

It totally ignores the previous logic and reads the request directly (and does it wrong).

So we don't have to implement printable-detection for situations where printable is true but not 'yes' since it's undocumented, unused and already removed in the current trunk.

Status & tagging log