r25050 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r25049‎ \| r25050 \| r25051 >
Date:	13:50, 22 August 2007
Author:	tstarling
Status:	old
Tags:
Comment:	* Fix handling of $linkAttribs=false * UI tweaks, not yet finalised: (none) option in player select, icon for audio * (Bug 10950) Leave some vertical space in embedded objects for controls, for plugins where controls are likely to be added. * Use innerHTML extensively, since document.createElement('object') is broken on several browsers.
Modified paths:	/trunk/extensions/OggHandler/OggHandler.i18n.php (modified) (history) /trunk/extensions/OggHandler/OggHandler.php (modified) (history) /trunk/extensions/OggHandler/OggHandler_body.php (modified) (history) /trunk/extensions/OggHandler/OggPlayer.js (modified) (history)

Diff [purge]

Index: trunk/extensions/OggHandler/OggHandler.php
—	—	@@ -19,5 +19,7 @@
20	20	$wgFFmpegLocation = 'ffmpeg';
21	21	$wgExtensionMessagesFiles['OggHandler'] = "$oggDir/OggHandler.i18n.php";
22	22	$wgParserOutputHooks['OggHandler'] = array( 'OggHandler', 'outputHook' );
	23	+$wgCortadoJarFile = "cortado-ovt-debug-0.2.2.1-patched.jar";
	24	+#$wgCortadoJarFile = "cortado-ovt-stripped-0.2.2.jar";
23	25
24	26	?>
Index: trunk/extensions/OggHandler/OggPlayer.js
—	—	@@ -65,6 +65,7 @@
66	66	break;
67	67	default:
68	68	elt.innerHTML = this.msg['ogg-no-player'] + '<br/>';
	69	+ player = 'none';
69	70	}
70	71	if ( this.showPlayerSelect ) {
71	72	elt.appendChild( document.createElement( 'br' ) );
—	—	@@ -116,17 +117,37 @@
117	118	this.clientSupports['cortado'] = navigator.javaEnabled();
118	119	},
119	120
120		~~- 'makePlayerSelect' : function ( selectedPlayer, id, videoUrl, width, height, length ) {~~
121		~~- var select = document.createElement( 'select' );~~
122		~~- for ( var player in this.clientSupports ) {~~
	121	+ 'addOption' : function ( select, value, text, selected ) {
123	122	var option = document.createElement( 'option' );
124		~~- option.value = player;~~
125		~~- option.appendChild( document.createTextNode( this.msg['ogg-player-' + player] ) );~~
126		~~- if ( selectedPlayer == player ) {~~
	123	+ option.value = value;
	124	+ option.appendChild( document.createTextNode( text ) );
	125	+ if ( selected ) {
127	126	option.selected = true;
128	127	}
129	128	select.appendChild( option );
	129	+ },
	130	+
	131	+ 'hx' : function ( s ) {
	132	+ if ( typeof s != 'String' ) {
	133	+ s = s.toString();
130	134	}
	135	+ return s.replace( /&/g, '&' )
	136	+ . replace( /</g, '<' )
	137	+ . replace( />/g, '>' );
	138	+ },
	139	+
	140	+ 'hq' : function ( s ) {
	141	+ return '"' + this.hx( s ) + '"';
	142	+ },
	143	+
	144	+ 'makePlayerSelect' : function ( selectedPlayer, id, videoUrl, width, height, length ) {
	145	+ var select = document.createElement( 'select' );
	146	+ if ( selectedPlayer == 'none' ) {
	147	+ this.addOption( select, 'none', this.msg['ogg-player-none'], true );
	148	+ }
	149	+ for ( var player in this.clientSupports ) {
	150	+ this.addOption( select, player, this.msg['ogg-player-' + player], selectedPlayer == player );
	151	+ }
131	152	select.value = selectedPlayer;
132	153
133	154	var me = this;
—	—	@@ -160,16 +181,16 @@
161	182
162	183	'embedVideoElement': function ( elt, videoUrl, width, height, length ) {
163	184	var videoElt = document.createElement('video');
164		~~- videoElt.setAttribute('width', width);~~
165		~~- videoElt.setAttribute('height', height);~~
166		~~- videoElt.setAttribute('src', videoUrl);~~
167		~~- videoElt.setAttribute('autoplay', '1');~~
168		~~- videoElt.setAttribute('controls', '1');~~
169		~~- elt.appendChild(videoElt);~~
	185	+ videoElt.setAttribute( 'width', width );
	186	+ videoElt.setAttribute( 'height', height + 20 );
	187	+ videoElt.setAttribute( 'src', videoUrl );
	188	+ videoElt.setAttribute( 'autoplay', '1' );
	189	+ videoElt.setAttribute( 'controls', '1' );
	190	+ elt.appendChild( videoElt );
170	191
171	192	// Try to detect implementations that don't support controls
172	193	// This works for the Opera test build
173		~~- if (!videoElt.controls) {~~
	194	+ if ( !videoElt.controls ) {
174	195	elt.appendChild( document.createElement( 'br' ) );
175	196	elt.appendChild( this.newPlayButton( videoElt ) );
176	197	elt.appendChild( this.newPauseButton( videoElt ) );
—	—	@@ -179,21 +200,25 @@
180	201	},
181	202
182	203	'embedOggPlugin': function ( elt, videoUrl, width, height, length ) {
183		~~- var videoElt = document.createElement( 'object' );~~
184		~~- videoElt.setAttribute( 'type', 'application/ogg' );~~
185		~~- videoElt.setAttribute( 'width', width );~~
186		~~- videoElt.setAttribute( 'height', height );~~
187		~~- videoElt.setAttribute( 'data', videoUrl );~~
188		~~- elt.appendChild(videoElt);~~
	204	+ var id = elt.id + "_obj";
	205	+ elt.innerHTML +=
	206	+ "<object id=" + this.hq( id ) +
	207	+ " type='application/ogg'" +
	208	+ " width=" + this.hq( width ) +
	209	+ " height=" + this.hq( height + 20 ) +
	210	+ " data=" + this.hq( videoUrl ) + "></object>";
189	211	},
190	212
191	213	'embedVlcPlugin' : function ( elt, videoUrl, width, height, length ) {
192		~~- var videoElt = document.createElement( 'object' );~~
193		~~- videoElt.setAttribute( 'type', 'application/x-vlc-plugin' );~~
194		~~- videoElt.setAttribute( 'width', width );~~
195		~~- videoElt.setAttribute( 'height', height );~~
196		~~- videoElt.setAttribute( 'data', videoUrl );~~
197		~~- elt.appendChild(videoElt);~~
	214	+ var id = elt.id + "_obj";
	215	+ elt.innerHTML +=
	216	+ "<object id=" + this.hq( id ) +
	217	+ " type='application/x-vlc-plugin'" +
	218	+ " width=" + this.hq( width ) +
	219	+ " height=" + this.hq( height ) +
	220	+ " data=" + this.hq( videoUrl ) + "></object>";
	221	+
	222	+ var videoElt = document.getElementById( id );
198	223	elt.appendChild( document.createElement( 'br' ) );
199	224	// TODO: seek bar
200	225	elt.appendChild( this.newPlayButton( videoElt ) );
—	—	@@ -208,13 +233,13 @@
209	234	// create this object with DOM functions. If anyone knows a better way
210	235	// than innerHTML, please let me know. -- TS
211	236	elt.innerHTML +=
212		~~- "<object id='" + id + "'" +~~
213		~~- " classid='clsid:9BE31822-FDAD-461B-AD51-BE1D1C159921'" +~~
214		~~- " codebase='http://downloads.videolan.org/pub/videolan/vlc/latest/win32/axvlc.cab#Version=0,8,6,0'" +~~
215		~~- " width='" + width + "'" +~~
216		~~- " height='" + height + "'>" +~~
217		~~- "<param name='mrl' value='" + videoUrl.replace(/&/, '&') + "'/>" +~~
218		~~- "</object>" ;~~
	237	+ '<object id=' + this.hq( id ) +
	238	+ ' classid="clsid:9BE31822-FDAD-461B-AD51-BE1D1C159921"' +
	239	+ ' codebase="http://downloads.videolan.org/pub/videolan/vlc/latest/win32/axvlc.cab#Version=0,8,6,0"' +
	240	+ ' width=' + this.hq( width ) +
	241	+ ' height=' + this.hq( height ) + ">" +
	242	+ '<param name="mrl" value=' + this.hq( videoUrl ) + '/>' +
	243	+ '</object>' ;
219	244
220	245	var videoElt = document.getElementById( id );
221	246	elt.appendChild( document.createElement( 'br' ) );
—	—	@@ -225,29 +250,34 @@
226	251	},
227	252
228	253	'embedCortado' : function ( elt, videoUrl, width, height, length ) {
229		~~- var videoElt;~~
230		~~- if ( false ) {~~
231		~~- // Use <object>~~
232		~~- videoElt = document.createElement( 'object' );~~
233		~~- videoElt.setAttribute( 'codetype', 'application/x-java-applet' );~~
234		~~- videoElt.setAttribute( 'classid', 'com.fluendo.player.Cortado.class' );~~
235		~~- } else {~~
236		~~- // Use <applet>~~
237		~~- videoElt = document.createElement( 'applet' );~~
238		~~- videoElt.setAttribute( 'code', 'com.fluendo.player.Cortado.class' );~~
	254	+ // Create the applet all at once
	255	+ // In Opera, document.createElement('applet') immediately creates
	256	+ // a non-working applet with unchangeable parameters, similar to the
	257	+ // problem with IE and ActiveX.
	258	+ elt.innerHTML =
	259	+ '<applet code="com.fluendo.player.Cortado.class" ' +
	260	+ ' width=' + this.hq( width ) +
	261	+ ' height=' + this.hq( height + 18 ) +
	262	+ ' archive=' + this.hq( this.cortadoUrl ) + '>' +
	263	+ ' <param name="url" value=' + this.hq( videoUrl ) + '/>' +
	264	+ ' <param name="duration" value=' + this.hq( length ) + '/>' +
	265	+ ' <param name="seekable" value="true"/>' +
	266	+ ' <param name="autoPlay" value="true"/>' +
	267	+ ' <param name="showStatus" value="show"/>' +
	268	+ ' <param name="statusHeight" value="18"/>' +
	269	+ '</applet>';
	270	+
	271	+ // Disable autoPlay in the DOM right now, to prevent Mozilla from
	272	+ // restarting an arbitrary number of applet instances on a back button click.
	273	+ // Unfortunately this means that some clients (e.g. Opera) won't autoplay at all
	274	+ var videoElt = elt.getElementsByTagName( 'applet' )[0];
	275	+ var params = videoElt.getElementsByTagName( 'param' );
	276	+ for ( var i = 0; i < params.length; i++ ) {
	277	+ if ( params[i].name == 'autoPlay' ) {
	278	+ params[i].value = '';
	279	+ break;
	280	+ }
239	281	}
240		~~- videoElt.setAttribute( 'width', width );~~
241		~~- videoElt.setAttribute( 'height', height );~~
242		~~- videoElt.setAttribute( 'archive', this.cortadoUrl );~~
243		-
244		~~- var param = document.createElement( 'param' );~~
245		~~- this.addParam( videoElt, 'url', videoUrl );~~
246		~~- this.addParam( videoElt, 'duration', length );~~
247		~~- this.addParam( videoElt, 'seekable', 'true' );~~
248		~~- this.addParam( videoElt, 'autoPlay', 'true' );~~
249		~~- this.addParam( videoElt, 'showStatus', 'show' );~~
250		~~- this.addParam( videoElt, 'statusHeight', 18 );~~
251		~~- elt.appendChild( videoElt );~~
252	282	},
253	283
254	284	'addParam': function ( elt, name, value ) {
—	—	@@ -257,6 +287,6 @@
258	288	elt.appendChild( param );
259	289	}
260	290	};
261		-
262	291
	292	+// vim: ts=4 sw=4 noet cindent :
263	293
Index: trunk/extensions/OggHandler/OggHandler_body.php
—	—	@@ -136,13 +136,14 @@
137	137
138	138	if ( $srcHeight == 0 \|\| $srcWidth == 0 ) {
139	139	// Make audio player
	140	+ $icon = $file->iconThumb();
140	141	if ( empty( $params['width'] ) ) {
141	142	$width = 200;
142	143	} else {
143	144	$width = $params['width'];
144	145	}
145		~~- $height = 20;~~
146		~~- return new OggAudioDisplay( $file->getURL(), $width, $height, $length );~~
	146	+ $height = $icon->getHeight();
	147	+ return new OggAudioDisplay( $file->getURL(), $icon->getUrl(), $width, $height, $length );
147	148	}
148	149
149	150	if ( $flags & self::TRANSFORM_LATER ) {
—	—	@@ -308,7 +309,7 @@
309	310	}
310	311
311	312	function setHeaders( $out ) {
312		~~- global $wgScriptPath;~~
	313	+ global $wgScriptPath, $wgCortadoJarFile;
313	314	if ( $out->hasHeadItem( 'OggHandler' ) ) {
314	315	return;
315	316	}
—	—	@@ -317,10 +318,10 @@
318	319
319	320	$msgNames = array( 'ogg-play', 'ogg-pause', 'ogg-stop', 'ogg-no-player',
320	321	'ogg-player-videoElement', 'ogg-player-oggPlugin', 'ogg-player-cortado', 'ogg-player-vlcPlugin',
321		~~- 'ogg-player-vlcActiveX', 'ogg-using-player' );~~
	322	+ 'ogg-player-vlcActiveX', 'ogg-player-none', 'ogg-using-player' );
322	323	$msgValues = array_map( 'wfMsg', $msgNames );
323	324	$jsMsgs = Xml::encodeJsVar( (object)array_combine( $msgNames, $msgValues ) );
324		~~- $encCortadoUrl = Xml::encodeJsVar( "$wgScriptPath/extensions/OggHandler/cortado-ovt-stripped-0.2.2.jar" );~~
	325	+ $encCortadoUrl = Xml::encodeJsVar( "$wgScriptPath/extensions/OggHandler/$wgCortadoJarFile" );
325	326
326	327	$out->addHeadItem( 'OggHandler', <<<EOT
327	328	<script type="text/javascript" src="$wgScriptPath/extensions/OggHandler/OggPlayer.js"></script>
—	—	@@ -355,19 +356,19 @@
356	357
357	358	function __construct( $videoUrl, $thumbUrl, $width, $height, $length, $isVideo ) {
358	359	$this->videoUrl = $videoUrl;
359		~~- $this->thumbUrl = $thumbUrl;~~
	360	+ $this->url = $thumbUrl;
360	361	$this->width = round( $width );
361	362	$this->height = round( $height );
362	363	$this->length = round( $length );
363	364	$this->isVideo = $isVideo;
364	365	}
365	366
366		~~- function toHtml( $attribs = array() , $linkAttribs = array() ) {~~
	367	+ function toHtml( $attribs = array() , $linkAttribs = false ) {
367	368	wfLoadExtensionMessages( 'OggHandler' );
368	369
369	370	OggTransformOutput::$serial++;
370	371
371		~~- $encThumbUrl = htmlspecialchars( $this->thumbUrl );~~
	372	+ $encThumbUrl = htmlspecialchars( $this->url );
372	373
373	374	if ( substr( $this->videoUrl, 0, 4 ) != 'http' ) {
374	375	global $wgServer;
—	—	@@ -379,30 +380,31 @@
380	381	$length = intval( $this->length );
381	382	$width = intval( $this->width );
382	383	$height = intval( $this->height );
	384	+ $attribs['src'] = $this->url;
383	385	if ( $this->isVideo ) {
384	386	$msgStartPlayer = wfMsg( 'ogg-play-video' );
385		~~- $thumb =~~
386		~~- Xml::tags( 'a', $linkAttribs,~~
387		~~- Xml::element( 'img',~~
388		~~- array(~~
389		~~- 'src' => $this->thumbUrl,~~
390		~~- 'width' => $width,~~
391		~~- 'height' => $height,~~
392		~~- ) + $attribs,~~
393		~~- null )~~
394		~~- ) .~~
395		~~- "<br/>\n";~~
	387	+ $attribs['width'] = $width;
	388	+ $attribs['height'] = $height;
	389	+ $playerHeight = $height;
396	390	} else {
397	391	$msgStartPlayer = wfMsg( 'ogg-play-sound' );
398		~~- $thumb = '';~~
	392	+ $playerHeight = 0;
	393	+ // Don't add width and height to the icon image, it won't match its true size
399	394	}
	395	+
	396	+ $thumb = Xml::element( 'img', $attribs, null );
	397	+ if ( $linkAttribs ) {
	398	+ $thumb = Xml::tags( 'a', $linkAttribs, $thumb );
	399	+ }
	400	+ $thumb .= "<br/>\n";
	401	+
400	402	$id = "ogg_player_" . OggTransformOutput::$serial;
401	403
402	404	$s = Xml::tags( 'div', array( 'id' => $id ),
403	405	$thumb .
404	406	Xml::element( 'button',
405	407	array(
406		~~- 'onclick' => "wgOggPlayer.init(false, '$id', $encUrl, $width, $height, $length);",~~
	408	+ 'onclick' => "wgOggPlayer.init(false, '$id', $encUrl, $width, $playerHeight, $length);",
407	409	),
408	410	$msgStartPlayer
409	411	)
—	—	@@ -418,8 +420,8 @@
419	421	}
420	422
421	423	class OggAudioDisplay extends OggTransformOutput {
422		~~- function __construct( $videoUrl, $width, $height, $length ) {~~
423		~~- parent::__construct( $videoUrl, false, $width, $height, $length, false );~~
	424	+ function __construct( $videoUrl, $iconUrl, $width, $height, $length ) {
	425	+ parent::__construct( $videoUrl, $iconUrl, $width, $height, $length, false );
424	426	}
425	427	}
426	428
Index: trunk/extensions/OggHandler/OggHandler.i18n.php
—	—	@@ -15,12 +15,14 @@
16	16	'ogg-stop' => 'Stop',
17	17	'ogg-play-video' => 'Play video',
18	18	'ogg-play-sound' => 'Play sound',
19		~~- 'ogg-no-player' => 'Sorry, no video player is available',~~
	19	+ 'ogg-no-player' => 'Sorry, your system does not appear to have any supported player software. ' .
	20	+ 'Please install <a href="http://www.java.com/en/download/manual.jsp">Java</a>.',
20	21	'ogg-player-videoElement' => '<video> element',
21	22	'ogg-player-oggPlugin' => 'Ogg plugin',
22	23	'ogg-player-cortado' => 'Cortado (Java)',
23	24	'ogg-player-vlcPlugin' => 'VLC (Mozilla)',
24	25	'ogg-player-vlcActiveX' => 'VLC (ActiveX)',
	26	+ 'ogg-player-none' => '(none)',
25	27	'ogg-using-player' => 'Using player: ',
26	28	),
27	29

Status & tagging log

15:20, 12 September 2011 Meno25 (talk | contribs) changed the status of r25050 [removed: ok added: old]