r107066 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r107065‎ | r107066 | r107067 >
Date:13:35, 22 December 2011
Author:hashar
Status:reverted (Comments)
Tags:
Comment:
apply CSS::Janus when user preview its CSS style

follow up r94421
Modified paths:
  • /trunk/phase3/includes/OutputPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/OutputPage.php
——@@ -3187,7 +3187,7 @@
31883188 array( 'excludepage' => $this->getTitle()->getPrefixedDBkey() )
31893189 );
31903190 // Load the previewed CSS
3191 - $otherTags .= Html::inlineStyle( $this->getRequest()->getText( 'wpTextbox1' ) );
 3191+ $this->addInlineStyle( $this->getRequest()->getText( 'wpTextbox1' ) );
31923192 } else {
31933193 // Load the user styles normally
31943194 $moduleStyles[] = 'user';

Follow-up revisions

RevisionCommit summaryAuthorDate
r108053Revert r107066, see CR comments for full rationale. Basically this moves the ...catrope17:15, 4 January 2012
r108078Redo r107066 properly: apply CSSJanus to user CSS previews when needed. Ping ...catrope20:01, 4 January 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r94421(bug 26283) Previewing user JS/CSS pages doesn't load other user JS/CSS pagescatrope17:27, 13 August 2011

Comments

#Comment by Nikerabbit (talk | contribs)   14:01, 22 December 2011

This doesn't cause any problems wrt the order the styles are loaded?

#Comment by Catrope (talk | contribs)   20:52, 22 December 2011

I think it just might. I'll have to take a closer look at this in the new year.

#Comment by Catrope (talk | contribs)   17:14, 4 January 2012

This doesn't actually trigger CSSJanus, look at the addInlineStyle() code: 

	/**
	 * Adds inline CSS styles
	 * @param $style_css Mixed: inline CSS
	 * @param $flip String: Set to 'flip' to flip the CSS if needed
	 */
	public function addInlineStyle( $style_css, $flip = 'noflip' ) {
		if( $flip === 'flip' && $this->getLanguage()->isRTL() ) {
			# If wanted, and the interface is right-to-left, flip the CSS
			$style_css = CSSJanus::transform( $style_css, true, false );
		}
		$this->mInlineStyles .= Html::inlineStyle( $style_css );
	}

Also, the rules for when to apply Janusing to site/user styles are different. Software styles are assumed to be LTR, so they are flipped if the interface is RTL. However, site/user styles are assumed to have a directionality that matches the content language (i.e. are assumed LTR if $wgLanguageCode is an LTR language, and RTL if it's an RTL language), so they're flipped if the interface language and the content language have different directionalities (viewing a wiki with LTR content in an RTL interface or vice versa).

Thirdly, this change loads the previewed CSS in the wrong place (too early -- user CSS is one of the last things in the head), as Nikerabbit suggested. So because it's breaking the loading order while not actually fixing anything (Janus behavior is unchanged), I'm gonna revert this. I'll look at a better way to handle Janusing properly.

Status & tagging log