Base layer change

[arfa]

When the user change the base layer (TileLayer) through the layer control, the loader does not appear for the first time the layer is loading. then when the tile layer loaded all visible tiles everything works fine.

I have added baselayerchange support in the method _addMapListeners().

_addMapListeners: function(map) {
    // Add listeners to the map for (custom) dataloading and dataload
    // events, eg, for AJAX calls that affect the map but will not be
    // reflected in the above layer events.
    map.on({
        baselayerchange: this._handleLoading,
        dataloading: this._handleLoading,
        dataload: this._handleLoad,
        layerremove: this._handleLoad
    }, this);
}

_removeMapListeners: function(map) {
    map.off({
        baselayerchange: this._handleLoading,
        dataloading: this._handleLoading,
        dataload: this._handleLoad,
        layerremove: this._handleLoad
    }, this);
}

This make the loader appears when the base layer is changed through the layer control, but I'm not sure it's the right way to fix this problem.

[ebrelsford]

Thanks for the issue! Do you have an example page available that reproduces this behavior? I agree that we should support this. And can you submit a PR?

I haven't looked into it yet but my only hesitation is that doing it this way might be a problem if the tiles in the new base layer are cached (so load would never be emitted for the layer). But maybe that won't actually happen?

[ebrelsford]

Nevermind, I added an example that does this and added your suggested change. It appears that load is emitted very quickly if the tiles are cached, so my fear seems to be incorrect. Please let me know if I am wrong.

[arfa]

hello @ebrelsford,

Thanks for your response, but it seems it is not the right event name. you have used baseLayerChange but all the word must be lowercase baselayerchange.

Luckily, what you have added does not make any difference. In fact I made some debbuging and I discover that this produces a serious problem of memory leakage.

When the user change the base layer two event are triggered layeradd and layerremove But in _addLayerListeners() only layeradd is treated, so every time the user change the base layer, new listeners are attached to the layer over and over.

_layerAdd: function(e) {
    if (!e.layer || !e.layer.on) return
    try {
        e.layer.on({
            loading: this._handleLoading,
            load: this._handleLoad
        }, this);
    } catch (exception) {
        console.warn('L.Control.Loading: Tried and failed to add ' +
            ' event handlers to layer', e.layer);
        console.warn('L.Control.Loading: Full details', exception);
    }
}

_addLayerListeners: function(map) {
    // Add listeners for begin and end of load to any layers already on the 
    // map
    map.eachLayer(function(layer) {
        if (!layer.on) return;
        layer.on({
            loading: this._handleLoading,
            load: this._handleLoad
        }, this);
    }, this);

    // When a layer is added to the map, add listeners for begin and end
    // of load
    map.on('layeradd', this._layerAdd, this);
}

I suggest you restore the old code until we find how to treat this problem.

[ebrelsford]

Fair enough, I'll fix the capitalization.

I'm not sure how the memory leak you're mentioning here is related to the change I made, however. That seems like a separate issue. We can add a symmetric _layerRemove that removes those event listeners from the layer when it's removed. Right?

[arfa]

Thanks, this fixes the problem.