DPS911 Release 3

This release I worked on Butter and TestSwarm for iOS.

Bug 256: Round timecodes

I had this in review before, and since then it has gone through a series of changes. Even after it was staged. My earlier post on this bug showed that I created a function, round, that would round a number given to it (round is underscored, because that’s how private variable are denoted now in Butter). The code now looks like this:

_round = function( number, numberOfDecimalPlaces ) {
  return Math.round( number * ( Math.pow( 10, numberOfDecimalPlaces ) ) ) / Math.pow( 10, numberOfDecimalPlaces );
};

In order to use the function, every place that calls the _round function has to pass in a number, and I kept passing in 3 every time. As Bobby suggested, this should be made into a const. So at the top of the file, I declared it as

const NUMBER_OF_DECIMAL_PLACES = 3;

because I saw it done that way in several other places in the code. But Jon Buckley noted that const is only supported on Firefox and Chrome, and linked to this page. So I changed the const to var, and also created another ticket to change all consts in Butter to vars.

After this was staged, Matthew Schranz found that when moving a track event by dragging and dropping, the 3 decimals were not being properly shown. This issue came up because the rounding function doesn’t add 0s to the end of a number. For example, a number like 12.44 will remain as 12.44, and will not become 12.440. We also decided to make the round function available to other places in the code. I ended up adding the function as a static function on Butter, and also made numberOfDecimalPlaces a static member on Butter, instead of an argument to the round function. This way the number of decimal places can be set in the code.

To fix the rounding issue Matthew was seeing, in the default editor, I added this code:

if ( item === "start" || item === "end" ) {
  newItem.value = popcornOptions[ item ].toFixed( 3 );
}
else {
  newItem.value = popcornOptions[ item ];
}

Taking advantage of the fact that the start and end times in the text fields are presented as strings, and that the toFixed returns a string, I used the toFixed function on the start and end times. That way, a start time of 0 will show up as 0.000, and a start time of 1.2 will show up as 1.200, giving a consistent view to users. I submitted another pull request, and Bobby gave me some review. I now have to

  • change numberOfDecimalPlaces to timeAccuracy
  • move the round function into util/time, instead of a static function on Butter

Bug 410: Change consts to vars

As noted earlier, I created another ticket to change all occurrences of const to var. This has already been staged, and the pull request for that (as well as the diff) is here.

Bug 390: Validate start and end times

Currently, a user can enter in a start or end time for a track event that is longer than the duration of the video. Since this doesn’t make sense, we need to show the user a message when it happens. One possible solution is to do the validation in the track event editor, but this only masks the problems. If another editor is created down the line, or if a track event is edited somewhere else in the code, then this bug will show up again. I decided to add this validation in the update function in the src/core/trackevent.js file. This way, no matter where the code is being updated from, the start and end times will be validated.

The update function now looks like this:

this.update = function( updateOptions ) {
  var errorMessage;
  if ( updateOptions.start >= _round( butter.duration, NUMBER_OF_DECIMAL_PLACES ) ) {
	errorMessage = "The in time cannot be greater than or equal to the duration of the video, which is " + _round( butter.duration, NUMBER_OF_DECIMAL_PLACES ) + " seconds.";
	_em.dispatch( "trackeventupdatefailed", errorMessage );
	return;
  } //if
  if ( updateOptions.end > _round( butter.duration, NUMBER_OF_DECIMAL_PLACES ) ) {
	errorMessage = "The out time cannot be greater than the duration of the video, which is " + _round( butter.duration, NUMBER_OF_DECIMAL_PLACES ) + " seconds.";
	_em.dispatch( "trackeventupdatefailed", errorMessage );
	return;
  } //if

  for ( var prop in updateOptions ) {
	if ( updateOptions.hasOwnProperty( prop ) ) {
	  _popcornOptions[ prop ] = updateOptions[ prop ];
	} //if
  } //for
  if ( _popcornOptions.start ) {
	_popcornOptions.start = _round( _popcornOptions.start, NUMBER_OF_DECIMAL_PLACES );
  }
  if ( _popcornOptions.end ) {
	_popcornOptions.end = _round( _popcornOptions.end, NUMBER_OF_DECIMAL_PLACES );
  }
  _em.dispatch( "trackeventupdated", _this );
}; //update

This function now checks for the errors at the top of the function. If it finds out, it sends out a trackeventupdatefailed failed event with a message, and returns early. Returning early here means the code that updates and sends the trackeventupdated event will not run, meaning that all of the code that listens and reacts to the trackeventupdated event will not run.

In order for this to work, some code has to be added to src/editor/editor.js so that it can send the trackeventupdatefailed event to the track event editor. In this file, once a trackeventupdatefailed is received, it runs this code:

butter.dialog.send( _dialogName, "trackeventupdatefailed", e.data );

Finally, the editor listens for trackeventupdatefailed, and simply displays the message to the user:

_comm.listen( "trackeventupdatefailed", function( e ) {
  document.getElementById( "message" ).innerHTML = e.data;
});

This is good because now when a new check is added for the track event in src/core/trackevent.js the error messages will automatically be shown if necessary in the editor. Bobby liked this approach, and wants the solution to either line up or replace another bug, bug 313, which is to make sure that the start and end times are actually valid numbers. The pull request and diff for this commit is here. I have feedback on it, which says

  • the butter variable is only global for debugging
  • store the rounded value of the video’s duration into a variable, instead of calculating it every time

TestSwarm for Popcorn

As stated in my blog post DPS911 Release 1, “we need a way to let the videos needed for Popcorn’s test cases to play automatically, without someone having to sit there and hit the play button”.

David Seifried fixed the issue of the web page not providing content to the iPhone device and simulator. The issue now is that tests run on a phsyical device, but on the simulator it simply says that there are no jobs left in the queue. This is going to require investigating. As I was writing this section of the blog, I think I found the solution to the problem of videos not playing unless a user clicks play. The UIWebView class has a mediaPlaybackRequiresUserAction variable that I can set to no, which will let videos play automatically! I will test that out as soon as we can get the simulator to start running tests.

Advertisements
This entry was posted in Open Source. Bookmark the permalink.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s