r61071 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r61070‎ | r61071 | r61072 >
Date:01:16, 15 January 2010
Author:simetrical
Status:resolved (Comments)
Tags:
Comment:
Merge all skins' output of opening <body> tag

This fixes a few minor discrepancies, like Vector outputting dir=""
(redundant to the one on <html>), and non-Monobook-based skins omitting
the capitalize-all-nouns class (!). This adds Html::openElement() and
refactors Html::rawElement() accordingly, so I checked that all parser
tests still pass.

I wasn't able to figure out if I broke some feature of right-floating
quickbars in the Standard skin, because I wasn't able to figure out what
the feature was in the first place. Hopefully either it works, or
nobody cares, or someone else will figure out what it was supposed to
do. (This is the stuff in getBodyOptions() in Standard.php I deleted;
I'm not sure the addition to sticky.js does what I want.)
Modified paths:
  • /trunk/phase3/includes/Html.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/skins/Modern.php (modified) (history)
  • /trunk/phase3/skins/MonoBook.php (modified) (history)
  • /trunk/phase3/skins/Standard.php (modified) (history)
  • /trunk/phase3/skins/Vector.php (modified) (history)
  • /trunk/phase3/skins/common/oldshared.css (modified) (history)
  • /trunk/phase3/skins/common/sticky.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/Standard.php
@@ -59,23 +59,6 @@
6060 return $s;
6161 }
6262
63 - /**
64 - *
65 - */
66 - function getBodyOptions() {
67 - $a = parent::getBodyOptions();
68 -
69 - if ( 3 == $this->qbSetting() ) { # Floating left
70 - $qb = "setup(\"quickbar\")";
71 - if( $a['onload'] ) {
72 - $a['onload'] .= ";$qb";
73 - } else {
74 - $a['onload'] = $qb;
75 - }
76 - }
77 - return $a;
78 - }
79 -
8063 function doAfterContent() {
8164 global $wgContLang, $wgLang;
8265 wfProfileIn( __METHOD__ );
Index: trunk/phase3/skins/common/sticky.js
@@ -31,7 +31,9 @@
3232 //mySticky.css.visibility="visible";
3333 }
3434
 35+hookEvent( 'load', function() { setup( 'quickbar' ); } );
3536
 37+
3638 // -------------------------
3739 // emulate css 'position: fixed' in IE5+ Win
3840 // code by aclover@1value.com
Index: trunk/phase3/skins/common/oldshared.css
@@ -374,3 +374,10 @@
375375 form#specialpages {
376376 display: inline;
377377 }
 378+
 379+body {
 380+ background-color: #ffffec;
 381+}
 382+body.ns-0 {
 383+ background-color: white;
 384+}
Index: trunk/phase3/skins/Vector.php
@@ -448,7 +448,6 @@
449449 // Output HTML Page
450450 $this->html( 'headelement' );
451451 ?>
452 - <body<?php if ( $this->data['body_ondblclick'] ): ?> ondblclick="<?php $this->text( 'body_ondblclick' ) ?>"<?php endif; ?> class="mediawiki <?php $this->text( 'dir' ) ?> <?php $this->text( 'pageclass' ) ?> <?php $this->text( 'skinnameclass' ) ?>" dir="<?php $this->text( 'dir' ) ?>">
453452 <div id="page-base" class="noprint"></div>
454453 <div id="head-base" class="noprint"></div>
455454 <!-- content -->
Index: trunk/phase3/skins/MonoBook.php
@@ -69,9 +69,7 @@
7070 wfSuppressWarnings();
7171
7272 $this->html( 'headelement' );
73 -?><body<?php if($this->data['body_ondblclick']) { ?> ondblclick="<?php $this->text('body_ondblclick') ?>"<?php } ?>
74 - class="mediawiki <?php $this->text('dir'); $this->text('capitalizeallnouns') ?> <?php $this->text('pageclass') ?> <?php $this->text('skinnameclass') ?>">
75 - <div id="globalWrapper">
 73+?> <div id="globalWrapper">
7674 <div id="column-content">
7775 <div id="content" <?php $this->html("specialpageattributes") ?>>
7876 <a id="top"></a>
Index: trunk/phase3/skins/Modern.php
@@ -62,8 +62,7 @@
6363 wfSuppressWarnings();
6464
6565 $this->html( 'headelement' );
66 -?><body<?php if($this->data['body_ondblclick']) { ?> ondblclick="<?php $this->text('body_ondblclick') ?>"<?php } ?>
67 - class="mediawiki <?php $this->text('dir') ?> <?php $this->text('pageclass') ?> <?php $this->text('skinnameclass') ?>">
 66+?>
6867
6968 <!-- heading -->
7069 <div id="mw_header"><h1 id="firstHeading"><?php $this->html('title') ?></h1></div>
Index: trunk/phase3/includes/OutputPage.php
@@ -1604,6 +1604,7 @@
16051605 global $wgDocType, $wgDTD, $wgContLanguageCode, $wgOutputEncoding, $wgMimeType;
16061606 global $wgXhtmlDefaultNamespace, $wgXhtmlNamespaces, $wgHtml5Version;
16071607 global $wgContLang, $wgUseTrackbacks, $wgStyleVersion, $wgHtml5, $wgWellFormedXml;
 1608+ global $wgUser, $wgRequest, $wgLang;
16081609
16091610 $this->addMeta( "http:Content-Type", "$wgMimeType; charset={$wgOutputEncoding}" );
16101611 if ( $sk->commonPrintStylesheet() ) {
@@ -1665,6 +1666,38 @@
16661667 $ret .= $this->getTitle()->trackbackRDF();
16671668
16681669 $ret .= "</head>\n";
 1670+
 1671+ $bodyAttrs = array();
 1672+
 1673+ # Crazy edit-on-double-click stuff
 1674+ $action = $wgRequest->getVal( 'action', 'view' );
 1675+
 1676+ if ( $this->mTitle->getNamespace() != NS_SPECIAL
 1677+ && !in_array( $action, array( 'edit', 'submit' ) )
 1678+ && $wgUser->getOption( 'editondblclick' ) ) {
 1679+ $bodyAttrs['ondblclick'] = "document.location = '" . Xml::escapeJsString( $this->mTitle->getEditURL() ) . "'";
 1680+ }
 1681+
 1682+ # Class bloat
 1683+ $bodyAttrs['class'] = "mediawiki $dir";
 1684+
 1685+ if ( $wgLang->capitalizeAllNouns() ) {
 1686+ # A <body> class is probably not the best way to do this . . .
 1687+ $bodyAttrs['class'] .= ' capitalize-all-nouns';
 1688+ }
 1689+ $bodyAttrs['class'] .= ' ns-' . $this->mTitle->getNamespace();
 1690+ if ( $this->mTitle->getNamespace() == NS_SPECIAL ) {
 1691+ $bodyAttrs['class'] .= ' ns-special';
 1692+ } elseif ( $this->mTitle->isTalkPage() ) {
 1693+ $bodyAttrs['class'] .= ' ns-talk';
 1694+ } else {
 1695+ $bodyAttrs['class'] .= ' ns-subject';
 1696+ }
 1697+ $bodyAttrs['class'] .= ' ' . Sanitizer::escapeClass( 'page-' . $this->mTitle->getPrefixedText() );
 1698+ $bodyAttrs['class'] .= ' skin-' . Sanitizer::escapeClass( $wgUser->getSkin()->getSkinName() );
 1699+
 1700+ $ret .= Html::openElement( 'body', $bodyAttrs ) . "\n";
 1701+
16691702 return $ret;
16701703 }
16711704
Index: trunk/phase3/includes/Html.php
@@ -106,6 +106,37 @@
107107 * @return string Raw HTML
108108 */
109109 public static function rawElement( $element, $attribs = array(), $contents = '' ) {
 110+ global $wgWellFormedXml;
 111+ $start = self::openElement( $element, $attribs );
 112+ if ( in_array( $element, self::$voidElements ) ) {
 113+ if ( $wgWellFormedXml ) {
 114+ # Silly XML.
 115+ return substr( $start, 0, -1 ) . ' />';
 116+ }
 117+ return $start;
 118+ } else {
 119+ return "$start$contents</$element>";
 120+ }
 121+ }
 122+
 123+ /**
 124+ * Identical to rawElement(), but HTML-escapes $contents (like
 125+ * Xml::element()).
 126+ */
 127+ public static function element( $element, $attribs = array(), $contents = '' ) {
 128+ return self::rawElement( $element, $attribs, strtr( $contents, array(
 129+ # There's no point in escaping quotes, >, etc. in the contents of
 130+ # elements.
 131+ '&' => '&amp;',
 132+ '<' => '&lt;'
 133+ ) ) );
 134+ }
 135+
 136+ /**
 137+ * Identical to rawElement(), but has no third parameter and omits the end
 138+ * tag (and the self-closing / in XML mode for empty elements).
 139+ */
 140+ public static function openElement( $element, $attribs = array() ) {
110141 global $wgHtml5, $wgWellFormedXml;
111142 $attribs = (array)$attribs;
112143 # This is not required in HTML5, but let's do it anyway, for
@@ -155,32 +186,11 @@
156187 }
157188 }
158189
159 - $start = "<$element" . self::expandAttributes(
160 - self::dropDefaults( $element, $attribs ) );
161 - if ( in_array( $element, self::$voidElements ) ) {
162 - if ( $wgWellFormedXml ) {
163 - return "$start />";
164 - }
165 - return "$start>";
166 - } else {
167 - return "$start>$contents</$element>";
168 - }
 190+ return "<$element" . self::expandAttributes(
 191+ self::dropDefaults( $element, $attribs ) ) . '>';
169192 }
170193
171194 /**
172 - * Identical to rawElement(), but HTML-escapes $contents (like
173 - * Xml::element()).
174 - */
175 - public static function element( $element, $attribs = array(), $contents = '' ) {
176 - return self::rawElement( $element, $attribs, strtr( $contents, array(
177 - # There's no point in escaping quotes, >, etc. in the contents of
178 - # elements.
179 - '&' => '&amp;',
180 - '<' => '&lt;'
181 - ) ) );
182 - }
183 -
184 - /**
185195 * Given an element name and an associative array of element attributes,
186196 * return an array that is functionally identical to the input array, but
187197 * possibly smaller. In particular, attributes might be stripped if they
Index: trunk/phase3/includes/SkinTemplate.php
@@ -485,13 +485,6 @@
486486 $content_actions = $this->buildContentActionUrls();
487487 $tpl->setRef( 'content_actions', $content_actions );
488488
489 - // XXX: attach this from javascript, same with section editing
490 - if( $this->iseditable && $wgUser->getOption( 'editondblclick' ) ){
491 - $encEditUrl = Xml::escapeJsString( $this->mTitle->getLocalUrl( $this->editUrlOptions() ) );
492 - $tpl->set( 'body_ondblclick', 'document.location = "' . $encEditUrl . '";' );
493 - } else {
494 - $tpl->set( 'body_ondblclick', false );
495 - }
496489 $tpl->set( 'sidebar', $this->buildSidebar() );
497490 $tpl->set( 'nav_urls', $this->buildNavUrls() );
498491
Index: trunk/phase3/includes/Skin.php
@@ -303,12 +303,6 @@
304304
305305 $out->out( $out->headElement( $this ) );
306306
307 - $out->out( "\n<body" );
308 - $ops = $this->getBodyOptions();
309 - foreach ( $ops as $name => $val ) {
310 - $out->out( " $name='$val'" );
311 - }
312 - $out->out( ">\n" );
313307 if ( $wgDebugComments ) {
314308 $out->out( "<!-- Wiki debugging output:\n" .
315309 $out->mDebugtext . "-->\n" );
@@ -657,29 +651,6 @@
658652 $out->addStyle( 'common/common_rtl.css', '', '', 'rtl' );
659653 }
660654
661 - function getBodyOptions() {
662 - global $wgUser, $wgOut, $wgRequest, $wgContLang;
663 -
664 - extract( $wgRequest->getValues( 'oldid', 'redirect', 'diff' ) );
665 -
666 - if ( 0 != $this->mTitle->getNamespace() ) {
667 - $a = array( 'bgcolor' => '#ffffec' );
668 - }
669 - else $a = array( 'bgcolor' => '#FFFFFF' );
670 - if( $wgOut->isArticle() && $wgUser->getOption( 'editondblclick' ) &&
671 - $this->mTitle->quickUserCan( 'edit' ) ) {
672 - $s = $this->mTitle->getFullURL( $this->editUrlOptions() );
673 - $s = 'document.location = "' .Xml::escapeJsString( $s ) .'";';
674 - $a += array( 'ondblclick' => $s );
675 - }
676 - $a['class'] =
677 - 'mediawiki' .
678 - ' '.( $wgContLang->getDir() ).
679 - ' '.$this->getPageClasses( $this->mTitle ) .
680 - ' skin-'. Sanitizer::escapeClass( $this->getSkinName() );
681 - return $a;
682 - }
683 -
684655 function getPageClasses( $title ) {
685656 $numeric = 'ns-'.$title->getNamespace();
686657 if( $title->getNamespace() == NS_SPECIAL ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r61158Use getTitle() instead of mTitle...simetrical01:47, 17 January 2010
r61556Removed the nasty-looking JS-based position:fixed simulation, used by the "st...tstarling03:19, 27 January 2010
r81604(bug 23315) Add new body classes to allow easier styling of special pages....vyznev21:08, 6 February 2011
r85583* (bug 28444) Fix regression: edit-on-doubleclick retains revision id again...brion21:58, 6 April 2011

Comments

#Comment by Raymond (talk | contribs)   06:34, 16 January 2010

Seen at translatewiki:

PHP Fatal error: Call to a member function getNamespace() on a non-object in /var/www/w/includes/OutputPage.php on line 1669

#Comment by 😂 (talk | contribs)   13:54, 16 January 2010

Might be worth using getTitle(), since it falls back to $wgTitle if $mTitle isn't set.

#Comment by Simetrical (talk | contribs)   01:48, 17 January 2010

Done in r61158. Raymond, is this resolved?

#Comment by Raymond (talk | contribs)   07:05, 17 January 2010

It seems so :) Thanks.

#Comment by Tim Starling (talk | contribs)   07:13, 26 January 2010

Both the float right and float left quickbar options seem to be broken in IE 6, before and after this change, even if I revert back to r60930. But they work in Firefox which has a working position:fixed and so doesn't need sticky.js. It's an obscure feature in an obscure browser, maybe we can just delete sticky.js now?

#Comment by Simetrical (talk | contribs)   14:34, 26 January 2010

Fine by me.

#Comment by Brion VIBBER (talk | contribs)   21:58, 6 April 2011

There was a regression here on old revision views: double-click started sending you to the current revision instead of the revision you were viewing. This is resolved on trunk in r85583.

Status & tagging log