prototype: Change Abstract.TimedObserver to subclass PeriodicalExecuter and tweak its subclasses to use new inheritance functionality. TimedObserver can now be stopped the same way as PeriodicalExecuter. Closes #8589.

CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Change Abstract.TimedObserver to subclass PeriodicalExecuter and tweak its subclasses to use new inheritance functionality.  TimedObserver can now be stopped the same way as PeriodicalExecuter.  Closes #8589.  [Mislav Marohnić]
+
 * Fix Class.mixin to extend the class's prototype.  [Mislav Marohnić]
 
 * Fix superclass method call breakage from [7337].  [Mislav Marohnić, sam]

src/base.js

@@ -241,6 +241,10 @@ var PeriodicalExecuter = Class.create({
   registerCallback: function() {
     this.timer = setInterval(this.onTimerEvent.bind(this), this.frequency * 1000);
   },
+
+  execute: function() {
+    this.callback(this);
+  },
   
   stop: function() {
     if (!this.timer) return;
@@ -252,7 +256,7 @@ var PeriodicalExecuter = Class.create({
     if (!this.currentlyExecuting) {
       try {
         this.currentlyExecuting = true;
-        this.callback(this);
+        this.execute();
       } finally {
         this.currentlyExecuting = false;
       }

src/form.js

@@ -257,41 +257,30 @@ Form.Element.Serializers = {
 
 /*--------------------------------------------------------------------------*/
 
-Abstract.TimedObserver = function() { };
-Abstract.TimedObserver.prototype = {
-  initialize: function(element, frequency, callback) {
-    this.frequency = frequency;
+Abstract.TimedObserver = Class.create(PeriodicalExecuter, {
+  initialize: function($super, element, frequency, callback) {
+    $super(callback, frequency);
     this.element   = $(element);
-    this.callback  = callback;
-    
     this.lastValue = this.getValue();
-    this.registerCallback();
   },
   
-  registerCallback: function() {
-    setInterval(this.onTimerEvent.bind(this), this.frequency * 1000);
-  },
-  
-  onTimerEvent: function() {
+  execute: function() {
     var value = this.getValue();
-    var changed = (Object.isString(this.lastValue) && Object.isString(value)
-      ? this.lastValue != value : String(this.lastValue) != String(value));
-    if (changed) {
+    if (Object.isString(this.lastValue) && Object.isString(value) ?
+        this.lastValue != value : String(this.lastValue) != String(value)) {
       this.callback(this.element, value);
       this.lastValue = value;
     }
   }
-};
+});
 
-Form.Element.Observer = Class.create();
-Form.Element.Observer.prototype = Object.extend(new Abstract.TimedObserver(), {
+Form.Element.Observer = Class.create(Abstract.TimedObserver, {
   getValue: function() {
     return Form.Element.getValue(this.element);
   }
 });
 
-Form.Observer = Class.create();
-Form.Observer.prototype = Object.extend(new Abstract.TimedObserver(), {
+Form.Observer = Class.create(Abstract.TimedObserver, {
   getValue: function() {
     return Form.serialize(this.element);
   }
@@ -299,8 +288,7 @@ Form.Observer.prototype = Object.extend(new Abstract.TimedObserver(), {
 
 /*--------------------------------------------------------------------------*/
 
-Abstract.EventObserver = function() { };
-Abstract.EventObserver.prototype = {
+Abstract.EventObserver = Class.create({
   initialize: function(element, callback) {
     this.element  = $(element);
     this.callback = callback;
@@ -321,7 +309,7 @@ Abstract.EventObserver.prototype = {
   },
   
   registerFormCallbacks: function() {
-    Form.getElements(this.element).each(this.registerCallback.bind(this));
+    Form.getElements(this.element).each(this.registerCallback, this);
   },
   
   registerCallback: function(element) {
@@ -337,17 +325,15 @@ Abstract.EventObserver.prototype = {
       }
     }    
   }
-};
+});
 
-Form.Element.EventObserver = Class.create();
-Form.Element.EventObserver.prototype = Object.extend(new Abstract.EventObserver(), {
+Form.Element.EventObserver = Class.create(Abstract.EventObserver, {
   getValue: function() {
     return Form.Element.getValue(this.element);
   }
 });
 
-Form.EventObserver = Class.create();
-Form.EventObserver.prototype = Object.extend(new Abstract.EventObserver(), {
+Form.EventObserver = Class.create(Abstract.EventObserver, {
   getValue: function() {
     return Form.serialize(this.element);
   }

test/unit/base.html

@@ -34,14 +34,6 @@
     return '-' + this.name;
   };
 
-  var peEventCount = 0;
-  // peEventFired will stop the PeriodicalExecuter after 3 callbacks
-  function peEventFired(pe) {
-    if (++peEventCount>2) {
-      pe.stop();
-    }
-  }
-  
   var arg1 = 1;
   var arg2 = 2;
   var arg3 = 3;
@@ -288,10 +280,16 @@
     }},
     
     testPeriodicalExecuterStop: function() {with(this) {
-      new PeriodicalExecuter(peEventFired, 0.1);
+      var peEventCount = 0;
+      function peEventFired(pe) {
+        if (++peEventCount > 2) pe.stop();
+      }
       
-      wait(1000, function() {
-          assertEqual(3, peEventCount);
+      // peEventFired will stop the PeriodicalExecuter after 3 callbacks
+      new PeriodicalExecuter(peEventFired, 0.05);
+      
+      wait(600, function() {
+        assertEqual(3, peEventCount);
       });
     }},
 

test/unit/form.html

@@ -129,8 +129,6 @@
   <!-- Tests follow -->
   <script type="text/javascript" charset="utf-8">
   // <![CDATA[
-    var callbackCounter = 0;
-    var timedCounter = 0;
 
     // sweet sweet additional assertions
     Object.extend(Test.Unit.Testcase.prototype, {
@@ -161,6 +159,7 @@
     }},
     
     testFormElementEventObserver: function(){ with(this) {
+      var callbackCounter = 0;
       var observer = new Form.Element.EventObserver('input_enabled', function(){
         callbackCounter++;
       });
@@ -172,6 +171,7 @@
     }},
 
     testFormElementObserver: function(){ with(this) {
+      var timedCounter = 0;
       // First part: regular field
       var observer = new Form.Element.Observer('input_enabled', 0.5, function() {
         ++timedCounter;
@@ -194,7 +194,7 @@
         });
       });
 
-      // Second part: multiple-select! -- Fails before patch in #6593.
+      // Second part: multiple-select
       [1, 2, 3].each(function(index) {
         $('multiSel1_opt' + index).selected = (1 == index);
       });