r79090 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r79089‎ | r79090 | r79091 >
Date:23:23, 27 December 2010
Author:dale
Status:deferred (Comments)
Tags:
Comment:
* fixed issues with dblclick binding and click to play binding interfering with eachother
* stubs for refactored language selection widget
* added warning to embedPlayer about iPad configuration value
Modified paths:
  • /branches/MwEmbedStandAlone/modules/EmbedPlayer/mw.EmbedPlayer.js (modified) (history)
  • /branches/MwEmbedStandAlone/modules/EmbedPlayer/skins/mw.PlayerControlBuilder.js (modified) (history)
  • /branches/MwEmbedStandAlone/modules/MediaWikiSupport (added) (history)
  • /branches/MwEmbedStandAlone/modules/MediaWikiSupport/README (added) (history)
  • /branches/MwEmbedStandAlone/modules/MediaWikiSupport/jQueryPlugins (added) (history)
  • /branches/MwEmbedStandAlone/modules/MediaWikiSupport/jQueryPlugins/jquery.ui.combobox.js (added) (history)
  • /branches/MwEmbedStandAlone/modules/MediaWikiSupport/loader.js (added) (history)
  • /branches/MwEmbedStandAlone/modules/MediaWikiSupport/mw.LanguageSelect.js (added) (history)
  • /branches/MwEmbedStandAlone/modules/MiroSubs/MiroSubs.i18n.php (modified) (history)
  • /branches/MwEmbedStandAlone/modules/MiroSubs/loader.js (modified) (history)
  • /branches/MwEmbedStandAlone/modules/MiroSubs/mw.MiroSubsConfig.js (modified) (history)
  • /branches/MwEmbedStandAlone/modules/MiroSubs/mw.TranslateSubsVolunteerlet.js (added) (history)
  • /branches/MwEmbedStandAlone/modules/SwarmTransport/mw.SwarmTransport.js (modified) (history)
  • /branches/MwEmbedStandAlone/modules/TimedText/mw.TimedTextEdit.js (modified) (history)
  • /branches/MwEmbedStandAlone/mwEmbed.js (modified) (history)
  • /branches/MwEmbedStandAlone/mwEmbedFrame.php (modified) (history)

Diff [purge]

Index: branches/MwEmbedStandAlone/mwEmbedFrame.php
——@@ -73,7 +73,7 @@
7474 // ( uses kaltura standard entry_id/{entryId} request )
7575 // normalize to the REQUEST object
7676 // @@FIXME: this should be moved over to a kaltura specific iframe implementation
77 - if( $_SERVER['PATH_INFO'] ){
 77+ if( isset( $_SERVER['PATH_INFO'] ) ){
7878 $kalturaUrlMap = Array(
7979 'entry_id' => 'kentryid',
8080 'uiconf_id' => 'kuiconfid',
Index: branches/MwEmbedStandAlone/mwEmbed.js
——@@ -90,8 +90,36 @@
9191
9292 // Apply any pre-setup config:
9393 mw.setConfig( preMwEmbedConfig );
94 -
 94+
9595 /**
 96+ * Merge in a configuration value:
 97+ */
 98+ mw.mergeConfig = function( name, value ){
 99+ if( typeof name == 'object' ) {
 100+ $j.each( name, function( inx, val) {
 101+ mw.setConfig( inx, val );
 102+ });
 103+ return ;
 104+ }
 105+ // Check if we should "merge" the config
 106+ if( typeof value == 'object' && typeof mwConfig[ name ] == 'object' ) {
 107+ if ( value.constructor.toString().indexOf("Array") != -1 &&
 108+ mwConfig[ name ].constructor.toString().indexOf("Array") != -1 ){
 109+ // merge in the array
 110+ mwConfig[ name ] = $j.merge( mwConfig[ name ], value );
 111+ } else {
 112+ for( var i in value ){
 113+ mwConfig[ name ][ i ] = value[ i ];
 114+ }
 115+ }
 116+ return ;
 117+ }
 118+ // else do a normal setConfig
 119+ mwConfig[ name ] = value;
 120+ mwNonDefaultConfigList.push( name );
 121+ };
 122+
 123+ /**
96124 * Set a default config value Will only update configuration if no value is
97125 * present
98126 *
Index: branches/MwEmbedStandAlone/modules/MediaWikiSupport/mw.LanguageSelect.js
——@@ -0,0 +1,16 @@
 2+/**
 3+ * Simple language selection uses.
 4+ */
 5+
 6+mw.LanguageSelect = {
 7+ /**
 8+ * Get the language box ( note this c
 9+ */
 10+ get: function( options ){
 11+ // Build a select object
 12+ // TODO test string construction instead of jQuery build out for performance
 13+ $j('<select id="combobox">
 14+
 15+ return $langSelect;
 16+ }
 17+};
\ No newline at end of file
Property changes on: branches/MwEmbedStandAlone/modules/MediaWikiSupport/mw.LanguageSelect.js
___________________________________________________________________
Added: svn:mime-type
118 + text/plain
Index: branches/MwEmbedStandAlone/modules/MediaWikiSupport/jQueryPlugins/jquery.ui.combobox.js
——@@ -0,0 +1,100 @@
 2+// Ui combobox copied:
 3+// http://jqueryui.com/demos/autocomplete/#combobox
 4+(function( $ ) {
 5+ $.widget( "ui.combobox", {
 6+ _create: function() {
 7+ var self = this,
 8+ select = this.element.hide(),
 9+ selected = select.children( ":selected" ),
 10+ value = selected.val() ? selected.text() : "";
 11+ var input = this.input = $( "<input>" )
 12+ .insertAfter( select )
 13+ .val( value )
 14+ .autocomplete({
 15+ delay: 0,
 16+ minLength: 0,
 17+ source: function( request, response ) {
 18+ var matcher = new RegExp( $.ui.autocomplete.escapeRegex(request.term), "i" );
 19+ response( select.children( "option" ).map(function() {
 20+ var text = $( this ).text();
 21+ if ( this.value && ( !request.term || matcher.test(text) ) )
 22+ return {
 23+ label: text.replace(
 24+ new RegExp(
 25+ "(?![^&;]+;)(?!<[^<>]*)(" +
 26+ $.ui.autocomplete.escapeRegex(request.term) +
 27+ ")(?![^<>]*>)(?![^&;]+;)", "gi"
 28+ ), "<strong>$1</strong>" ),
 29+ value: text,
 30+ option: this
 31+ };
 32+ }) );
 33+ },
 34+ select: function( event, ui ) {
 35+ ui.item.option.selected = true;
 36+ self._trigger( "selected", event, {
 37+ item: ui.item.option
 38+ });
 39+ },
 40+ change: function( event, ui ) {
 41+ if ( !ui.item ) {
 42+ var matcher = new RegExp( "^" + $.ui.autocomplete.escapeRegex( $(this).val() ) + "$", "i" ),
 43+ valid = false;
 44+ select.children( "option" ).each(function() {
 45+ if ( $( this ).text().match( matcher ) ) {
 46+ this.selected = valid = true;
 47+ return false;
 48+ }
 49+ });
 50+ if ( !valid ) {
 51+ // remove invalid value, as it didn't match anything
 52+ $( this ).val( "" );
 53+ select.val( "" );
 54+ input.data( "autocomplete" ).term = "";
 55+ return false;
 56+ }
 57+ }
 58+ }
 59+ })
 60+ .addClass( "ui-widget ui-widget-content ui-corner-left" );
 61+
 62+ input.data( "autocomplete" )._renderItem = function( ul, item ) {
 63+ return $( "<li></li>" )
 64+ .data( "item.autocomplete", item )
 65+ .append( "<a>" + item.label + "</a>" )
 66+ .appendTo( ul );
 67+ };
 68+
 69+ this.button = $( "<button>&nbsp;</button>" )
 70+ .attr( "tabIndex", -1 )
 71+ .attr( "title", "Show All Items" )
 72+ .insertAfter( input )
 73+ .button({
 74+ icons: {
 75+ primary: "ui-icon-triangle-1-s"
 76+ },
 77+ text: false
 78+ })
 79+ .removeClass( "ui-corner-all" )
 80+ .addClass( "ui-corner-right ui-button-icon" )
 81+ .click(function() {
 82+ // close if already visible
 83+ if ( input.autocomplete( "widget" ).is( ":visible" ) ) {
 84+ input.autocomplete( "close" );
 85+ return;
 86+ }
 87+
 88+ // pass empty string as value to search for, displaying all results
 89+ input.autocomplete( "search", "" );
 90+ input.focus();
 91+ });
 92+ },
 93+
 94+ destroy: function() {
 95+ this.input.remove();
 96+ this.button.remove();
 97+ this.element.show();
 98+ $.Widget.prototype.destroy.call( this );
 99+ }
 100+ });
 101+ })( jQuery );
Property changes on: branches/MwEmbedStandAlone/modules/MediaWikiSupport/jQueryPlugins/jquery.ui.combobox.js
___________________________________________________________________
Added: svn:mime-type
1102 + text/plain
Index: branches/MwEmbedStandAlone/modules/MediaWikiSupport/loader.js
——@@ -0,0 +1,9 @@
 2+
 3+// Wrap in mw
 4+( function( mw ) {
 5+ // Add as loader dependency 'mw.style.mirosubsMenu'
 6+ mw.addResourcePaths({
 7+ "mw.LanguageSelectBox" : "mw.LanguageSelectBox.js"
 8+ });
 9+
 10+} )( window.mw );
\ No newline at end of file
Property changes on: branches/MwEmbedStandAlone/modules/MediaWikiSupport/loader.js
___________________________________________________________________
Added: svn:mime-type
111 + text/plain
Index: branches/MwEmbedStandAlone/modules/MediaWikiSupport/README
——@@ -0,0 +1,4 @@
 2+This folder should contain all the mediaWiki specific helpers.
 3+
 4+* Related to the embedPlayer this allows you to use mediaWiki title keys to pull in video assets.
 5+* Any utilities that are not part of core resource loader components ( api helper )
Index: branches/MwEmbedStandAlone/modules/MiroSubs/mw.TranslateSubsVolunteerlet.js
——@@ -0,0 +1,12 @@
 2+/**
 3+* A volenteerlet is a small bit of code that can be invoked to do a small a few minutes of work
 4+*
 5+* I hope to generalize the concept into a separate module / extension, and let modules extend the
 6+* base volteerlet object with specific tasks.
 7+*
 8+* This voletnerlet is designed to help people rapidly crowd source subtitles translation.
 9+*/
 10+
 11+mw.TranslateSubsVolunteerlet = function(){
 12+
 13+};
\ No newline at end of file
Property changes on: branches/MwEmbedStandAlone/modules/MiroSubs/mw.TranslateSubsVolunteerlet.js
___________________________________________________________________
Added: svn:mime-type
114 + text/plain
Index: branches/MwEmbedStandAlone/modules/MiroSubs/loader.js
——@@ -43,12 +43,13 @@
4444 mw.addLoaderDialog( gM('mwe-mirosubs-loading-universal-subtitles') );
4545 // Load miro subs:
4646 mw.load( 'MiroSubs', function(){
 47+ // Close loader dialog
 48+ mw.closeLoaderDialog();
 49+
4750 // Open the mirosubs dialog:
48 - mw.MiroSubsConfig.openDialog( embedPlayer, function(){
49 - // dialog Ready close loader
50 - mw.closeLoaderDialog();
51 - });
 51+ mw.MiroSubsConfig.openDialog( embedPlayer );
5252 });
 53+ // don't follow the line item # link
5354 return false;
5455 })
5556 );
Index: branches/MwEmbedStandAlone/modules/MiroSubs/mw.MiroSubsConfig.js
——@@ -7,19 +7,14 @@
88 */
99 mw.MiroSubsConfig = {
1010 config : null,
11 - openDialog: function( embedPlayer, dialogReadyCallback ){
 11+ openDialog: function( embedPlayer ){
1212 var _this = this;
1313 this.getConfig( embedPlayer , function( config ){
1414 if( !config ){
1515 return ;
16 - }
17 - // xxx NOTE there are some weird async display issues
18 - // that only seem to be resolvable with timeouts for DOM actions
 16+ }
 17+ // Show the dialog ( wait 500ms because of weird async DOM issues with mirosub wiget
1918 setTimeout(function(){
20 - dialogReadyCallback();
21 - }, 100);
22 - // Show the dialog
23 - setTimeout(function(){
2419 _this.mirosubs = mirosubs.api.openDialog( config );
2520 }, 800);
2621 });
——@@ -44,11 +39,22 @@
4540
4641 // Check both the user name and subtitles have been set:
4742 var isConfigReady = function(){
48 - if( _this.config.username && _this.config.subtitles ){
 43+ if( _this.config.username
 44+ &&
 45+ _this.config.subtitles
 46+ &&
 47+ _this.config.languageKey
 48+ ){
4949 callback( _this.config );
5050 }
5151 };
5252
 53+ // Get the language selection from user input ( dialog )
 54+ _this.getContentLanguage( function( langKey ){
 55+ _this.config.languageKey = langKey;
 56+ isConfigReady();
 57+ });
 58+
5359 // Make sure we are logged in::
5460 mw.getUserName( function( userName ){
5561 mw.log( "MiroSubsConfig::getUserName: " + userName );
——@@ -71,6 +77,22 @@
7278 isConfigReady();
7379 });
7480 },
 81+
 82+ getContentLanguage: function(){
 83+ var $dialog = mw.addDialog( {
 84+ 'title' : gM("mwe-mirosubs-content-language"),
 85+ 'width' : 450,
 86+ 'content' : $j('<div />').append(
 87+ $j('<h3 />').text( gM("mwe-mirosubs-content-language") ),
 88+ $j('<input/>').attr({
 89+ 'id' : 'mwe-mirosubs-save-summary',
 90+ 'size': '35'
 91+ }).val( gM('mwe-mirosubs-save-default') )
 92+ ),
 93+ 'buttons' : buttons
 94+ });
 95+ },
 96+
7597 /**
7698 * Present a dialog to get the target language
7799 */
——@@ -84,9 +106,6 @@
85107 // By default the config status is 'ok'
86108 'status' : 'ok',
87109
88 - // Default language key 'en':
89 - 'languageKey' : 'en',
90 -
91110 'closeListener': function(){
92111 // close event refresh page?
93112 // make sure any close dialog is 'closed'
Index: branches/MwEmbedStandAlone/modules/MiroSubs/MiroSubs.i18n.php
——@@ -19,5 +19,6 @@
2020 'mwe-mirosubs-thankyou-contribution' => 'Thank you for your subtitle contribution',
2121 'mwe-mirosubs-subs-saved-error' => 'Error in saving subtitles',
2222 'mwe-mirosubs-subs-please-login' => 'Please login',
23 - 'mwe-mirosubs-subs-please-login-desc' => 'Please login, to use the universal subtitles editor'
 23+ 'mwe-mirosubs-subs-please-login-desc' => 'Please login, to use the universal subtitles editor',
 24+ 'mwe-mirosubs-content-language' => 'Set content language',
2425 );
\ No newline at end of file
Index: branches/MwEmbedStandAlone/modules/SwarmTransport/mw.SwarmTransport.js
——@@ -19,10 +19,14 @@
2020 // Confirm SwarmTransport add-on is available ( defines swarmTransport var )
2121 if( _this.getPluginLibrary() ){
2222 // Add the swarm source
23 - _this.addSwarmSource( embedPlayer, function(){
24 - // Update the source if paused
25 - if( embedPlayer.paused ) {
26 - embedPlayer.setupSourcePlayer();
 23+ _this.addSwarmSource( embedPlayer, function( status ){
 24+ // Check if the status returned true
 25+ if( status ){
 26+ // Update the source if paused
 27+ if( embedPlayer.paused ) {
 28+ // Resetup sources
 29+ embedPlayer.setupSourcePlayer();
 30+ }
2731 }
2832 });
2933 }
——@@ -116,7 +120,7 @@
117121 // Check if the torrent is ready:
118122 if( !data.torrent ){
119123 mw.log( "SwarmTransport: Torrent not ready status: " + data.status.text );
120 - callback();
 124+ callback( false );
121125 return ;
122126 }
123127 mw.log( 'SwarmTransport: addSwarmSource for: ' + source.getSrc() + "\n\nGot:" + data.torrent );
——@@ -131,7 +135,7 @@
132136 } )
133137 .get( 0 )
134138 );
135 - callback();
 139+ callback( true );
136140 }
137141 );
138142 },
Index: branches/MwEmbedStandAlone/modules/TimedText/mw.TimedTextEdit.js
——@@ -371,6 +371,7 @@
372372 });
373373 })
374374 },
 375+
375376 /**
376377 * Gets the language set.
377378 *
——@@ -400,10 +401,11 @@
401402 // call out to "anonymous" function to variable scope the langKey
402403 $langMenu.append(
403404 _this.getLangMenuItem( langKey , source_icon)
404 - )
 405+ );
405406 }
406407 return $langMenu;
407408 },
 409+
408410 getLangMenuItem: function( langKey , source_icon) {
409411 return $j.getLineItem(
410412 langKey + ' - ' + mw.Language.names[ langKey ],
Index: branches/MwEmbedStandAlone/modules/EmbedPlayer/mw.EmbedPlayer.js
——@@ -2374,6 +2374,10 @@
23752375 if( this.isPersistentNativePlayer() && mw.getConfig('EmbedPlayer.EnableIpadHTMLControls') ){
23762376 return false;
23772377 } else {
 2378+ // Set warning that your trying to do iPad controls without persistent native player:
 2379+ if( mw.getConfig('EmbedPlayer.EnableIpadHTMLControls') ){
 2380+ mw.log("Error:: Trying to set EnableIpadHTMLControls without Persistent Native Player");
 2381+ }
23782382 return true;
23792383 }
23802384 }
——@@ -2673,25 +2677,24 @@
26742678 }
26752679 }
26762680
2677 - // If we previously finished playing this clip run the "replay hook"
2678 - if( this.donePlayingCount > 0 && !this.paused) {
2679 - mw.log("replayEvent");
2680 - $j( this ).trigger( 'replayEvent' );
2681 - }
2682 -
26832681
26842682 if( this.paused === true ){
2685 - this.paused = false;
2686 -
 2683+ this.paused = false;
26872684 // Check if we should Trigger the play event
26882685 if( this.bubbleEventCheck() ) {
26892686 mw.log("EmbedPlayer:: trigger play even::" + !this.paused);
2690 - if( this._propagateEvents ){
 2687+ if( this._propagateEvents ){
26912688 $j( this ).trigger( 'play' );
26922689 _this.tempDisableEvents();
26932690 }
26942691 }
26952692 }
 2693+
 2694+ // If we previously finished playing this clip run the "replay hook"
 2695+ if( this.donePlayingCount > 0 && !this.paused) {
 2696+ mw.log("replayEvent");
 2697+ $j( this ).trigger( 'replayEvent' );
 2698+ }
26962699
26972700 this.$interface.find('.play-btn span')
26982701 .removeClass( 'ui-icon-play' )
——@@ -2737,7 +2740,7 @@
27382741 if( this.bubbleEventCheck() ){
27392742 mw.log('EmbedPlayer:trigger pause:' + this.paused);
27402743 if( this._propagateEvents ){
2741 - $j( this ).trigger('pause' );
 2744+ $j( this ).trigger( 'pause' );
27422745 _this.tempDisableEvents();
27432746 }
27442747 }
Index: branches/MwEmbedStandAlone/modules/EmbedPlayer/skins/mw.PlayerControlBuilder.js
——@@ -567,24 +567,41 @@
568568
569569 // Remove any old interface bindings
570570 $interface.unbind();
571 -
572 - // Play & Pause on Click
573 - var bindFirstPlay = false;
574 - $j(embedPlayer).bind('play', function() { //Only bind once played
 571+
 572+ var bindFirstPlay = false;
 573+ // Bind into play.ctrl namespace ( so we can unbind without affecting other play bindings )
 574+ $j(embedPlayer).unbind('play.ctrl').bind('play.ctrl', function() { //Only bind once played
575575 if(bindFirstPlay) {
576576 return ;
577577 }
578578 bindFirstPlay = true;
579 - $j(embedPlayer).click( function() {
580 - if(embedPlayer.getPlayerElement().controls) {
 579+ var dblClickTime = 300;
 580+ var lastClickTime = 0;
 581+ var didDblClick = false;
 582+ // Remove parent dbl click ( so we can handle play clicks )
 583+ $j( embedPlayer ).unbind("dblclick").click( function() {
 584+ // Don't bind anything if native controls displayed:
 585+ if( embedPlayer.getPlayerElement().controls ) {
581586 return ;
 587+ }
 588+ var clickTime = new Date().getTime();
 589+ if( clickTime -lastClickTime < dblClickTime ) {
 590+ embedPlayer.fullscreen();
 591+ didDblClick = true;
 592+ setTimeout( function(){ didDblClick = false; }, dblClickTime + 10 );
582593 }
 594+ lastClickTime = clickTime;
 595+ setTimeout( function(){
 596+ // check if no click has since the time we called the setTimeout
 597+ if( !didDblClick ){
 598+ if( embedPlayer.paused ) {
 599+ embedPlayer.play();
 600+ } else {
 601+ embedPlayer.pause();
 602+ }
 603+ }
 604+ }, dblClickTime );
583605
584 - if(embedPlayer.paused) {
585 - embedPlayer.play();
586 - } else {
587 - embedPlayer.pause();
588 - }
589606 });
590607 });
591608

Comments

#Comment by Krinkle (talk | contribs)   23:26, 27 December 2010

This looks unfinished: 

+		// TODO test string construction instead of jQuery build out for performance  
+		$j('<select id="combobox">
+
+		return $langSelect;

Aside from the TODO, the function call isn't complete (ie the missing .. ');)

#Comment by Mdale (talk | contribs)   23:37, 27 December 2010

thanks for the review .. as the commit line mentions its 'a stub' but fair to say it should be a non-broken stub, will be fixed in the next commit

#Comment by Mdale (talk | contribs)   22:24, 28 December 2010

fixed in r79136

Status & tagging log