Fix adding/removing events on an unstamped context

[danzel]

If you add an event listener with a context with no stamp, then stamp the context and try remove the event listener we fail to do so as we assume that we need to use the stamped array, which hasn't been set yet.

I've changed this to always stamp the context (if there is one), so we either go the stamped way, or the no context way.

Not sure why we weren't always just stamping the context before, so please have a check over @mourner @jfgirard thanks :-)

Fixes #1495, Fixes #1512
Refs #1141

[jfgirard]

Usually, the layer is already stamp by the map.addLayer function. But I think it's a good idea to make sure the context is always stamp like you did 👍

[iirvine]

@jfgirard right, issue was around adding a feature to a FeatureGroup before it was added to a map. Guarding with L.stamp() was essentially how I was working around it as well.

[iirvine]

I'll just tack this on here instead of opening another issue.

Ran into one more edge case today - calling the current implementation of Events#removeEventListener with a stamped context will decrement the objLenKey index for that particular event for all contexts, even if the current context isn't registered. this can lead to a sticky situation where the objLenKey index is at zero before other event listeners have been fully cleared out, leading to zombies.

it('can occasionally be fooled with multiple stamped contexts', function(){
            var obj = new Klass(),
                spy = jasmine.createSpy()
                spy2 = jasmine.createSpy(),
                foo = {},
                foo2 = {};

            L.Util.stamp(foo);
            L.Util.stamp(foo2);

            obj.addEventListener('test', spy, foo2);

            obj.removeEventListener('test', spy, foo); // Decrements test_idx to 0, even though event listener isn't registered with foo's _leaflet_id
            obj.removeEventListener('test', spy, foo2);  // Doesn't get removed

            obj.addEventListener('test', spy2, foo);

            obj.fireEvent('test');

            expect(spy).not.toHaveBeenCalled();
        });

fix is simply to change the conditional here if (contextId && listeners.length === 0 && events[objKey][contextId] !== undefined)

[danzel]

Awesome thanks, have added that one too.

[mourner]

We always stamp context in addEventListener so we can use L.stamp here too.
Also, please use spaces for alignment here.

[mourner]

OK, looks good! Thanks @danzel and @iirvine for the help!

[danzel]

Cool, changed to L.stamp there too. We are using spaces for alignment there, the old code must have been tabs.

[mourner]

Awesome, thanks Dave!