VP-210: Determine whether or not there is a bug in controllers relating to start time.

Description

On line 224 we are binding starttime with two different and opposite sounding functions. Let's take a closer look at this to see if it's reasonable.

Comments

• Michelle D'Souza commented 2012-04-16T12:00:01.353-0400

  var bindScrubberModel = function (that) {
  // Setup the scrubber when we know the duration of the video.
  that.applier.modelChanged.addListener("startTime", that.updateMin);
  that.applier.modelChanged.addListener("startTime", that.updateMax);
  that.applier.modelChanged.addListener("totalTime", that.updateMax);

• Anastasia Cheetham commented 2012-08-08T10:37:49.837-0400

  I've looked into this a bit, and as far as I can tell, the binding of 'startTime' to updateMax, while harmless, is unnecessary.

  The two update functions are independent in that they use the different times (start and end), with no sharing. The two events seem to only be fired when the 'durationchange' event is fired, which as far as I can tell only happens on load. Our code updates both start and end time on durationchange. I can't see any reason from the code why startTime should be bound to updateMax.

  I've also tested the code without the extra listener (IE9 and FF on mac), and it seems to have no effect: all tests still pass, and the behaviour seems identical.

• Cindy Li commented 2012-08-08T13:57:46.134-0400

  I agree firing updateMax by listening to "startTime" seems unnecessary. The goal of updateMax is to update totalTime on scrubber and elsewhere, which makes sense to be triggered by "totalTime" rather than "startTime".

• Michelle D'Souza commented 2012-08-10T13:42:21.322-0400

  Merged into project repo at 07b8fd08aa7a8c938e26dc55812f3087d32fd984