Simplify fakeStop/skipped code paths

[johnd0e]

Currently disableClickPropagation is rather complicated.

For some cases it uses fakeStop:

Leaflet/src/dom/DomEvent.js

Lines 257 to 260 in 436430d

	export function fakeStop(e) {
	// fakes stopPropagation by setting a special event flag, checked/reset with skipped(e)
	skipEvents[e.type] = true;
	}

The purpose of this function - to solve #301. Whether it is really best solution - is out of scope of this issue, but here I want to point the fact, that it is implemented in such way, that not easy to understand. There is also related skipped function with unobvious behavior, called from couple of places, and so on.

1. Originally it was rather simple: 5a7420d.
2. It was more complex after 5c1a349, which purpose was fix for IE<9.

I suppose we should not care about legacy IE anymore, and propose to revert latter commit.

[johnd0e]

May be it's possible to do it in compatible way: johnd0e/Leaflet@improve-DomEventSpec...johnd0e:enhance-disableClickPropagation

• disableClickPropagation can be implemented by attaching some leaflet property to DOM element, and than in _handleDOMEvent checking it.
• skipped is also used by stopPropagation and originated from here.
  The only purpose of skipped here - neutralize flaws of current limited implementation of disableClickPropagation.

  • It's possible that in some places it was the sole purpose of calling stopPropagation, so such calls should be removed.

    Leaflet/src/layer/Popup.js

    Line 485 in 436430d

    DomEvent.stop(e);
• fakeStop is also used in Canvas._onClick (originated from: 1, 2)
  It's sole purpose here - prevent double-processing of events (do not pass them to map). And again, the simpler solution would be just relying on DOM element property.
• Atm only canvas case is covered by tests. Edit Lacking tests added by DomEventSpec.js: add tests #7438.

[johnd0e]

In fact the things are even more complicated, as we also has similar facility in stopPropagation: it attaches ._stopped property (originated from 54c8925) to original event.

Then it's checked in _fireDOMEvent:

Leaflet/src/map/Map.js

Line 1415 in 436430d

if (e._stopped) { return; }

Leaflet/src/map/Map.js

Line 1441 in 436430d

if (data.originalEvent._stopped ||

Edit:
Ok, this serves different purpose. Leaflet has additional events processing, see #3307.

Edit 2:

But what I've found. See this line:

Leaflet/src/map/Map.js

Line 1415 in 436430d

if (e._stopped) { return; }

It's originated from #3917 (or #3307?), and the sole purpose of it - fix flaws of implementation of other feature - preventing events already processed by canvas.

After commenting out that line this test case breaks:

Canvas #events DOM events fired on canvas polygon can be cancelled before being caught by the map FAILED
        expected 1 to sort of equal 0
        assert@node_modules/expect.js/index.js:102:16
        eql@node_modules/expect.js/index.js:230:16
        spec/suites/layer/vector/CanvasSpec.js:59:35

After #7439 that is not needed anymore - no one of our tests make that condition to be true.

[johnd0e]

There is another place that is not clean enough:

Leaflet/src/dom/DomEvent.js

Lines 165 to 166 in 436430d

	} else if (e.originalEvent) { // In case of Leaflet event.
	e.originalEvent._stopped = true;

This property is for leaflet's own event, but why then it is added to originalEvent?

I've found the reason:

Leaflet/src/core/Events.js

Lines 174 to 181 in 436430d

	fire: function (type, data, propagate) {
	if (!this.listens(type, propagate)) { return this; }
	var event = Util.extend({}, data, {
	type: type,
	target: this,
	sourceTarget: data && data.sourceTarget || this
	});

As you can see - listener is unable to change original data object, because it receives only copy of it!
But that copy contains originalEvent object, same as in data, and that's make it possible to pass anything from listener.

Sort of hack.
May be better to think about #6787.