jenkins-bot has submitted this change and it was merged.
Change subject: EventEmitter tweaks
......................................................................
EventEmitter tweaks
* Prevent accidental object's function calling when emitting events.
* Add #one for callbacks that should be fired only once.
* Don't use $.extend( obj.prototype, EventEmitter.prorotype ), can be
bug prone (name clashes).
Change-Id: I11de9760fa7e6fe5a0ef57204321689e6fcc6a0b
---
M javascripts/common/eventemitter.js
M javascripts/common/mf-api.js
M javascripts/modules/mf-photo.js
M tests/javascripts/common/test_eventemitter.js
4 files changed, 55 insertions(+), 24 deletions(-)
Approvals:
Jdlrobson: Verified; Looks good to me, approved
jenkins-bot: Verified
diff --git a/javascripts/common/eventemitter.js
b/javascripts/common/eventemitter.js
index 783e580..33226b6 100644
--- a/javascripts/common/eventemitter.js
+++ b/javascripts/common/eventemitter.js
@@ -1,5 +1,12 @@
( function( M, $ ) {
+ function callbackProxy( callback ) {
+ return function() {
+ var args = Array.prototype.slice.call( arguments, 1 );
+ callback.apply( callback, args );
+ };
+ }
+
function EventEmitter() {}
/**
@@ -9,10 +16,18 @@
* @param {Function} callback Callback to be bound.
*/
EventEmitter.prototype.on = function( event, callback ) {
- $( this ).on( event, function() {
- var args = Array.prototype.slice.call( arguments, 1 );
- callback.apply( callback, args );
- } );
+ $( this ).on( event, callbackProxy( callback ) );
+ return this;
+ };
+
+ /**
+ * Bind a callback to the event and run it only once.
+ *
+ * @param {string} event Event name.
+ * @param {Function} callback Callback to be bound.
+ */
+ EventEmitter.prototype.one = function( event, callback ) {
+ $( this ).one( event, callbackProxy( callback ) );
return this;
};
@@ -24,7 +39,10 @@
*/
EventEmitter.prototype.emit = function( event /* , arg1, arg2, ... */ )
{
var args = Array.prototype.slice.call( arguments, 1 );
- $( this ).trigger( event, args );
+ // use .triggerHandler() for emitting events to avoid
accidentally
+ // invoking object's functions, e.g. don't call obj.something()
when
+ // doing obj.emit( 'something' )
+ $( this ).triggerHandler( event, args );
return this;
};
diff --git a/javascripts/common/mf-api.js b/javascripts/common/mf-api.js
index d6958fc..4e772e1 100644
--- a/javascripts/common/mf-api.js
+++ b/javascripts/common/mf-api.js
@@ -20,6 +20,8 @@
this.initialize( options );
}
+ Api.prototype = new EventEmitter();
+
// FIXME: make Api and View inherit from an abstract Class object
/**
* Constructor that can be overriden.
@@ -74,7 +76,9 @@
// need to bind this event before we open the
connection (see note at
//
https://developer.mozilla.org/en-US/docs/DOM/XMLHttpRequest/Using_XMLHttpRequest#Monitoring_progress)
xhr.upload.addEventListener( 'progress',
function( ev ) {
- request.emit( 'progress', ev );
+ if ( ev.lengthComputable ) {
+ self.emit( 'progress', request,
ev.loaded / ev.total );
+ }
} );
}
return xhr;
@@ -82,7 +86,6 @@
*/
request = $.ajax( options );
- $.extend( request, EventEmitter.prototype );
this.requests.push( request );
return request;
};
diff --git a/javascripts/modules/mf-photo.js b/javascripts/modules/mf-photo.js
index 09c81e7..c52c07f 100644
--- a/javascripts/modules/mf-photo.js
+++ b/javascripts/modules/mf-photo.js
@@ -5,7 +5,6 @@
CtaDrawer = M.require( 'CtaDrawer' ),
funnel = $.cookie( 'mwUploadsFunnel' ) || 'article',
showCta = mw.config.get( 'wgMFEnablePhotoUploadCTA' ) || funnel
=== 'nearby',
- EventEmitter = M.require( 'eventemitter' ),
ProgressBar = M.require( 'widgets/progress-bar' ),
Overlay = M.require( 'Overlay' ),
ownershipMessage = mw.msg( 'mobile-frontend-photo-ownership',
mw.config.get( 'wgUserName' ), mw.user ),
@@ -179,7 +178,8 @@
function doUpload( token ) {
var formData = new FormData(),
- ext = options.file.name.slice(
options.file.name.lastIndexOf( '.' ) + 1 );
+ ext = options.file.name.slice(
options.file.name.lastIndexOf( '.' ) + 1 ),
+ request;
options.fileName = generateFileName(
options.description, '.' + ext );
@@ -201,7 +201,7 @@
} )
);
- self.post( formData, {
+ request = self.post( formData, {
// iOS seems to ignore the cache
parameter so sending r parameter
// send useformat=mobile for sites
where endpoint is a desktop url so that they are mobile edit tagged
url: apiUrl + '?useformat=mobile&r=' +
Math.random(),
@@ -209,10 +209,6 @@
cache: false,
contentType: false,
processData: false
- } ).on( 'progress', function( ev ) {
- if ( ev.lengthComputable ) {
- self.emit( 'progress',
ev.loaded / ev.total );
- }
} ).done( function( data ) {
var descriptionUrl = '',
warnings = data.upload ?
data.upload.warnings : false;
@@ -253,7 +249,14 @@
result.reject( status + ': ' +
error );
}
} );
+
+ self.on( 'progress', function( req, progress ) {
+ if ( req === request ) {
+ self.emit( 'uploadProgress',
progress );
+ }
+ } );
}
+
self.getToken( 'edit', endpoint ).done( function( token
) {
doUpload( token );
} ).fail( function( err ) {
@@ -263,8 +266,6 @@
return result;
}
} );
-
- $.extend( PhotoApi.prototype, EventEmitter.prototype );
LearnMoreOverlay = Overlay.extend( {
defaults: {
@@ -666,7 +667,7 @@
self.emit( 'error' );
} );
- api.on( 'progress', function( value ) {
+ api.on( 'uploadProgress', function( value ) {
progressPopup.setValue( value );
} );
}
diff --git a/tests/javascripts/common/test_eventemitter.js
b/tests/javascripts/common/test_eventemitter.js
index 1193daf..a59bc61 100644
--- a/tests/javascripts/common/test_eventemitter.js
+++ b/tests/javascripts/common/test_eventemitter.js
@@ -1,14 +1,23 @@
( function( M ) {
- var EventEmitter = M.require( 'eventemitter' );
+ var EventEmitter = M.require( 'eventemitter' );
QUnit.module( 'MobileFrontend EventEmitter' );
- QUnit.test( '#on', 1, function() {
- var e = new EventEmitter(), spy = sinon.spy();
- e.on( 'testEvent', spy );
- e.emit( 'testEvent', 'first', 2 );
- ok( spy.calledWith( 'first', 2 ), 'run callback when event runs' );
- } );
+ QUnit.test( '#on', 1, function( assert ) {
+ var e = new EventEmitter(), spy = sinon.spy();
+ e.on( 'testEvent', spy );
+ e.emit( 'testEvent', 'first', 2 );
+ assert.ok( spy.calledWith( 'first', 2 ), 'run callback when
event runs' );
+ } );
+
+ QUnit.test( '#one', 2, function( assert ) {
+ var e = new EventEmitter(), spy = sinon.spy();
+ e.one( 'testEvent', spy );
+ e.emit( 'testEvent', 'first', 2 );
+ e.emit( 'testEvent', 'second', 2 );
+ assert.ok( spy.calledWith( 'first', 2 ), 'run callback when
event runs' );
+ assert.ok( spy.calledOnce, 'run callback once' );
+ } );
}( mw.mobileFrontend) );
--
To view, visit https://gerrit.wikimedia.org/r/68531
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I11de9760fa7e6fe5a0ef57204321689e6fcc6a0b
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: JGonera <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
