Re: [proximity] some more tests...

Hi Marcos,

On 25.5.2012, at 14.36, ext Marcos Caceres wrote:

> Hi,
> I've added the .ondeviceproximity and .onuserproximity tests to the test suite (test suite now contains 87 tests).
> 
> I've also implemented the .ondeviceproximity and .onuserproximity in the reference implementation.
> You can see it running here:
> http://jsfiddle.net/marcosc/r3JnV/
> (some test fails are expected, because there is no way to extend the Event host object)
> 
> Again, code review of tests and any feedback welcome:
> https://github.com/marcoscaceres/proximitysensor

This looks even better, great work!

One thing I noticed is that the polyfill is firing events even if there are no listeners registered (I remember you commented about this earlier so I checked how your polyfill behaves). This is really a QoI issue, and the spec is intentionally silent on this.

If you wish, you could augment addEventListener and start firing only when there are listeners registered, something like (using device proximity as an example):

  var p = Window.prototype;
  p._addEventListener = p.addEventListener;
  p.addEventListener = function (type, callback, capture) {
    if (type === 'deviceproximity') {
      // start firing events, if not already
    }
    this._addEventListener(type, callback, capture);
  };

... and stop firing when all the listeners are removed (and handle .ondeviceproximity similarly). That said, augmenting built-in objects can be risky, so this may not be worth it.

Btw. All this relates to the popular topic of addEventListener side effects ...

Anyway, thanks for the cool polyfill and the test suite!

-Anssi

Received on Friday, 25 May 2012 16:33:03 UTC