Fix passive event warning in chrome

[kaliatech]

Summary

On chrome, initializing babylon.js with standard camera input controls causes a warning in the console about a non-passive event listener related to wheel events. The warning reads like this:

babylon.js:16 [Violation] Added non-passive event listener to a scroll-blocking 'wheel' event. Consider marking event handler as 'passive' to make the page more responsive. See www.chromestatus.com/feature/5745543795965952

	e.attachControl	@	babylon.js:16
	t.attachControl	@	babylon.js:16
	t	@	babylon.js:16
	createScene	@	VM451:6
	initFunction	@	VM451:33

This happens with Babylon 5.x, 4.2.x, etc, and has likely been happening in Chrome for some time now. The warning can seemingly be safely ignored, but it is annoying and likely causes new users to wonder if something is wrong.

History

Around Chrome 52, support for passive event listeners was added. At some point later, Chrome started showing warnings when certain event listeners are added without an explicit passive setting. In this case, the warning is due to Babylon cameras listening to mouse wheel events. The intent by the chrome team is to notify web devs via warning so that unless preventDefault handling is actually needed, then passive=true can be used which allows optimization for default page scroll handling.

Passive Decision

I debated setting passive to true or false. I set it to false because that seems like a safer option. However, it be might be acceptable to set it to true depending on how/where the wheel event listener is attached in the DOM, AND how it relates to the CameraInputsManager.noPreventDefault setting.

The exceptional case is around expectations when using a mouse wheel on a scrollable page and with a Babylon camera. I would guess that most users expect the mouse wheel to control camera when mouse pointer is over the Babylon canvas, else wheel should scroll the page.

Setting passive to false replicates existing functionality (because that is the default), whatever that is. Making it explicit seems to be enough to prevent the warning in chrome.

Related Links

Related discussion in the three.js project:

• Support passive event listeners in controls mrdoob/three.js#10373

Related forum post

• https://forum.babylonjs.com/t/chrome-warning-about-non-passive-event-listener-for-wheel-event/17373

Supporting info:

• https://www.chromestatus.com/feature/5745543795965952
• https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener
• https://caniuse.com/passive-event-listener

[RaananW]

Thanks for that! I am all for fixing warnings.

We are still supporting IE11, will that not fail when running the code in IE? IE (seems to me, at least?) doesn't support an object as options (and only a boolean).

[deltakosh]

That needs to be tested but I'm confident as an object can be converted to true

@kaliatech : Do you mind testing on IE to be sure?

[kaliatech]

I'm attempting to test, but having problems getting a babylonjs dev setup that works for testing IE11. Will use forums to discuss if I'm unable to get it working.

[kaliatech]

RE: IE11

Testing the original change worked, as it was, in IE11 without any obvious issue. However, it is not obvious if IE11 is internally setting useCapture to true or false when given an object. I don't think safe to assume it treats as normal JS truthiness. BUT, if it did, it would be setting useCapture to true, which is not the default (I think) and not what it was previously.

I suspect that changing from the default would not affect anyone. The difference between event capturing and bubbling is very esoteric. (More info here.) But just-in-case, I added another commit with feature detection to handle IE11 and make sure it works exactly the way it did previously. Up to you if the extra code is worthwhile.

RE: Passive and CameraInputsManager.noPreventDefault

As I mentioned in original comment, I set passive to false because of Bablyon's handling of CameraInputsManager.noPreventDefault. If noPreventDefault is false, then a passive = true setting results in an error in chrome console like:

• Unable to preventDefault inside passive event listener invocation

...as expected. In theory, if noPreventDefault = true, then passive could be set to true and maybe there would be a small browser performance improvement. I didn't see any easy to handle that with existing code organization though, and I think unlikely it actually matters.

(As an aside, unclear why CameraInputsManager.noPreventDefault was used as the variable instead of the preventDefault. The sort-of double negative name seems confusing.)

Example

In case not obvious, all of this revolves around the scenario where there is a babylon.js canvas on the page, babylon is setup with a camera that makes use of the mouse wheel, and the page itself is scrollable.

In this example, noPreventDefault = true, and so when wheel is scrolled with mouse pointer over the canvas, the camera zooms in/out simultaneously with the web page itself scrolling:

Setting noPreventDefault = false fixes this so that wheel controls camera if pointer is on top of canvas, otherwise it scrolls page. And in that case, passive could have been set to true.

[deltakosh]

I'm fine with the changes then
Thanks for the great explanation

LGTM