r54695 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r54694‎ | r54695 | r54696 >
Date:02:06, 10 August 2009
Author:simetrical
Status:reverted (Comments)
Tags:
Comment:
Remove extraneous type="" for CSS/JS in HTML 5

Removed in a bunch of places, but of course there are zillions left!
This is one of the things that would be a lot easier if we could just
drop support for non-HTML 5 output.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/OutputPage.php
——@@ -109,7 +109,12 @@
110110 * addStyle() and draws from the /skins folder.
111111 */
112112 public function addExtensionStyle( $url ) {
113 - $linkarr = array( 'rel' => 'stylesheet', 'href' => $url, 'type' => 'text/css' );
 113+ global $wgHtml5;
 114+ $linkarr = array( 'rel' => 'stylesheet', 'href' => $url );
 115+ if ( !$wgHtml5 ) {
 116+ # type attribute is mandatory in XHTML 1.0 Transitional
 117+ $linkarr['type'] = 'text/css';
 118+ }
114119 array_push( $this->mExtStyles, $linkarr );
115120 }
116121
——@@ -118,7 +123,7 @@
119124 * @param string $file filename in skins/common or complete on-server path (/foo/bar.js)
120125 */
121126 function addScriptFile( $file ) {
122 - global $wgStylePath, $wgStyleVersion, $wgJsMimeType, $wgScript, $wgUser;
 127+ global $wgStylePath, $wgStyleVersion, $wgJsMimeType, $wgScript, $wgUser, $wgHtml5;
123128 global $wgJSAutoloadClasses, $wgJSAutoloadLocalClasses, $wgEnableScriptLoader, $wgScriptPath;
124129
125130 if( substr( $file, 0, 1 ) == '/' ) {
——@@ -163,15 +168,11 @@
164169 }
165170
166171 // if the script loader did not find a way to add the script than add using addScript
167 - $this->addScript(
168 - Xml::element( 'script',
169 - array(
170 - 'type' => $wgJsMimeType,
171 - 'src' => wfAppendQuery( $path, $this->getURIDparam() ),
172 - ),
173 - '', false
174 - )
175 - );
 172+ $attrs = array( 'src' => wfAppendQuery( $path, $this->getURIDparam() ) );
 173+ if ( !$wgHtml5 ) {
 174+ $attrs['type'] = $wgJsMimeType;
 175+ }
 176+ $this->addScript( Xml::element( 'script', $attrs, '', false ) );
176177 }
177178
178179 /**
——@@ -180,7 +181,8 @@
181182 * different page load types (edit, upload, view, etc)
182183 */
183184 function addCoreScripts2Top(){
184 - global $wgEnableScriptLoader, $wgStyleVersion, $wgJSAutoloadLocalClasses, $wgJsMimeType, $wgScriptPath, $wgEnableJS2system;
 185+ global $wgEnableScriptLoader, $wgStyleVersion, $wgJSAutoloadLocalClasses,
 186+ $wgJsMimeType, $wgScriptPath, $wgEnableJS2system, $wgHtml5;
185187 //@@todo we should deprecate wikibits in favor of mv_embed and native jQuery functions
186188
187189 if( $wgEnableJS2system ){
——@@ -195,12 +197,11 @@
196198 $so = '';
197199 foreach( $core_classes as $s ){
198200 if( isset( $wgJSAutoloadLocalClasses[$s] ) ){
199 - $so.= Xml::element( 'script', array(
200 - 'type' => $wgJsMimeType,
201 - 'src' => "{$wgScriptPath}/{$wgJSAutoloadLocalClasses[$s]}?" . $this->getURIDparam()
202 - ),
203 - '', false
204 - );
 201+ $attrs = array( 'src' => "{$wgScriptPath}/{$wgJSAutoloadLocalClasses[$s]}?" . $this->getURIDparam() );
 202+ if ( !$wgHtml5 ) {
 203+ $attrs['type'] = $wgJsMimeType;
 204+ }
 205+ $so.= Xml::element( 'script', $attrs, '', false );
205206 }
206207 }
207208 $this->mScripts = $so . $this->mScripts;
——@@ -213,7 +214,7 @@
214215 */
215216 function addScriptClass( $js_class ){
216217 global $wgDebugJavaScript, $wgJSAutoloadLocalClasses, $wgJSAutoloadClasses,
217 - $wgJsMimeType, $wgEnableScriptLoader, $wgStyleVersion, $wgScriptPath;
 218+ $wgJsMimeType, $wgEnableScriptLoader, $wgStyleVersion, $wgScriptPath, $wgHtml5;
218219
219220 if( isset( $wgJSAutoloadClasses[$js_class] ) || isset( $wgJSAutoloadLocalClasses[$js_class] ) ){
220221 if( $wgEnableScriptLoader ){
——@@ -228,16 +229,12 @@
229230 }else if( isset( $wgJSAutoloadLocalClasses[$js_class] ) ){
230231 $path.= $wgJSAutoloadLocalClasses[$js_class];
231232 }
232 - $urlApend = ( $wgDebugJavaScript) ? time() : $wgStyleVersion;
233 - $this->addScript(
234 - Xml::element( 'script',
235 - array(
236 - 'type' => $wgJsMimeType,
237 - 'src' => "$path?" . $urlApend,
238 - ),
239 - '', false
240 - )
241 - );
 233+ $urlAppend = ( $wgDebugJavaScript) ? time() : $wgStyleVersion;
 234+ $attrs = array( 'src' => "$path?$urlAppend" );
 235+ if ( !$wgHtml5 ) {
 236+ $attrs['type'] = $wgJsMimeType;
 237+ }
 238+ $this->addScript( Xml::element( 'script', $attrs, '', false ) );
242239 }
243240 return true;
244241 }
——@@ -250,7 +247,7 @@
251248 * @param $forcClassAry Boolean: false by default
252249 */
253250 function getScriptLoaderJs( $forceClassAry = false ){
254 - global $wgJsMimeType, $wgStyleVersion, $wgRequest, $wgDebugJavaScript;
 251+ global $wgJsMimeType, $wgStyleVersion, $wgRequest, $wgDebugJavaScript, $wgHtml5;
255252
256253 if( !$forceClassAry ){
257254 $class_list = implode( ',', $this->mScriptLoaderClassList );
——@@ -268,14 +265,11 @@
269266
270267 //generate the unique request param (combine with the most recent revision id of any wiki page with the $wgStyleVersion var)
271268
272 -
273 - return Xml::element( 'script',
274 - array(
275 - 'type' => $wgJsMimeType,
276 - 'src' => wfScript( 'mwScriptLoader' ) . "?class={$class_list}{$debug_param}&".$this->getURIDparam(),
277 - ),
278 - '', false
279 - );
 269+ $attrs = array( 'src' => wfScript( 'mwScriptLoader' ) . "?class={$class_list}{$debug_param}&".$this->getURIDparam() );
 270+ if ( !$wgHtml5 ) {
 271+ $attrs['type'] = $wgJsMimeType;
 272+ }
 273+ return Xml::element( 'script', $attrs, '', false );
280274 }
281275
282276 function getURIDparam(){
——@@ -304,8 +298,9 @@
305299 * @param string $script JavaScript text, no <script> tags
306300 */
307301 function addInlineScript( $script ) {
308 - global $wgJsMimeType;
309 - $this->mScripts .= "\t\t<script type=\"$wgJsMimeType\">/*<![CDATA[*/\n\t\t$script\n\t\t/*]]>*/</script>\n";
 302+ global $wgJsMimeType, $wgHtml5;
 303+ $this->mScripts .= "\t\t<script" . ($wgHtml5 ? '' : " type=\"$wgJsMimeType\"")
 304+ . ">/*<![CDATA[*/\n\t\t$script\n\t\t/*]]>*/</script>\n";
310305 }
311306
312307 function getScript() {
——@@ -1940,8 +1935,24 @@
19411936 return "\t" . implode( "\n\t", $links );
19421937 }
19431938
 1939+ /**
 1940+ * Return a <link rel="stylesheet"> with the given parameters.
 1941+ *
 1942+ * @param $style string Relative or absolute URL. If $style begins with
 1943+ * '/', 'http:', or 'https:' it's used for the href as-is. Otherwise,
 1944+ * it's assumed to refer to some file within $wgStylePath, so the URL
 1945+ * used is "$wgStylePath/$style?$wgStyleVersion".
 1946+ * @param $options array Associative array with the following possible
 1947+ * keys:
 1948+ * 'dir': 'rtl' or 'ltr'. If the site language direction doesn't match
 1949+ * this, the empty string is returned.
 1950+ * 'media': Contents of the media attribute (will be mangled a bit,
 1951+ * e.g. 'screen' -> 'screen,projection' so Opera fullscreen works)
 1952+ * 'condition': Wrap in an IE conditional comment, e.g., 'lt IE 7'.
 1953+ * @return string Raw HTML
 1954+ */
19441955 protected function styleLink( $style, $options ) {
1945 - global $wgRequest;
 1956+ global $wgRequest, $wgHtml5;
19461957
19471958 if( isset( $options['dir'] ) ) {
19481959 global $wgContLang;
——@@ -1970,9 +1981,11 @@
19711982
19721983 $attribs = array(
19731984 'rel' => 'stylesheet',
1974 - 'href' => $url,
1975 - 'type' => 'text/css' );
1976 - if( $media ) {
 1985+ 'href' => $url );
 1986+ if ( !$wgHtml5 ) {
 1987+ $attribs['type'] = 'text/css';
 1988+ }
 1989+ if ( $media ) {
19771990 $attribs['media'] = $media;
19781991 }
19791992
Index: trunk/phase3/includes/Skin.php
——@@ -333,9 +333,10 @@
334334 }
335335
336336 static function makeVariablesScript( $data ) {
337 - global $wgJsMimeType;
 337+ global $wgJsMimeType, $wgHtml5;
338338
339 - $r = array( "<script type=\"$wgJsMimeType\">/*<![CDATA[*/" );
 339+ $r = array( "<script" . ( $wgHtml5 ? '' : " type=\"$wgJsMimeType\"" )
 340+ . ">/*<![CDATA[*/" );
340341 foreach ( $data as $name => $value ) {
341342 $encValue = Xml::encodeJsVar( $value );
342343 $r[] = "var $name = $encValue;";
Index: trunk/phase3/RELEASE-NOTES
——@@ -198,6 +198,7 @@
199199 numbers outside the permitted ranges), etc.
200200 ** The summary attribute has been removed from tables of contents. summary is
201201 obsolete in HTML 5 and wasn't useful here anyway.
 202+** Remove unnecessary type="" for CSS/JS in HTML 5 output
202203 * New hook SpecialRandomBeforeSQL allows extensions to modify or replace the SQL
203204 query used in Special:Random and subclasses, deprecating the $wgExtraRandompageSQL
204205 config variable

Follow-up revisions

RevisionCommit summaryAuthorDate
r54734Reverting r54695 'Remove extraneous type="" for CSS/JS in HTML 5'...brion19:23, 10 August 2009

Comments

#Comment by Brion VIBBER (talk | contribs)   19:23, 10 August 2009

This creates massive code duplication, checking a var and either including or not including the type attribute. Instead, this should be consolidated into functions in the Xml class for building <style>, <script>, or <link rel="stylesheet"> elements.

#Comment by Simetrical (talk | contribs)   00:10, 11 August 2009

Implemented more sanely in r54767.

Status & tagging log