There are many cases where local functions or variables are being exposed as part of the mw.popups object. We should go over the codebase and make those functions and variables local.

From https://gerrit.wikimedia.org/r/#/c/313258/4/resources/ext.popups.renderer/desktopRenderer.js:

In a follow up can we take the opportunity to stop clickHandler and leaveInactive from being a global?
It's really confusing as it's only used in this file so no need to be exposed anywhere.

A/C

• Variables that can be local are not made global.

Notes on the use of globals

ext.popups.core

The following globals encourage the renderer to contain logic and to act like a controller (getting the information) and with some refactoring do not need to be global. Many are also exposed for testing purposes. We should reconsider what, why and how we test these things

• mw.popups.scrolled
• mw.popups.IGNORE_CLASSES
• mw.popups.removeTooltips - exposed for testing but shouldn't be a global
• mw.popups.setupTriggers - feels like this is part of an init method.
• mw.popups.getTitle
• mw.popups.api
• mw.popups.getAction
• mw.popups.getRandomToken - currently exposed by ext.popups.core but only used by ext.popups.schemaPopups.utils
• mw.popups.getPreviewCountBucket - currently exposed by ext.popups.core but only used by ext.popups.schemaPopups.utils
• mw.popups.incrementPreviewCount - currently exposed by ext.popups.core but only used by ext.popups.renderer.desktopRenderer

This should probably be moved to ext.popups.settings.js or their own dedicated module. isUserInCondition is only exposed for testing purposes.

• mw.popups.saveEnabledState
• mw.popups.getEnabledState
• mw.popups.isUserInCondition

ext.popups.targets.desktopTarget

All the following can be trivially changed:

• checkScroll
• createPopupElement
• createSVGMask

ext.popups.schemaPopups.utils

This appears to be exposed for the purpose of testing and for use inside ext.popups.schemaPopups We could use module.exports instead to avoid population of the global mw.popup and explore merging the two ResourceLoader modules.

• mw.popups.schemaPopups

ext.popups.desktop

Probably shouldn't expose any globals yet it exposes the following:

• mw.popups.render.abortCurrentRequest
• mw.popups.render.getClosestYPosition

It overrides mw.popups.render.renderers.article. it would be better to do something explicit such as mw.popups.registerRenderer

• mw.popups.render.renderers.article

ext.popups.renderer.desktopRenderer

openPopup and closePopup should arguably be the only globally exposed methods.

• mw.popups.render.POPUP_DELAY
• mw.popups.render.POPUP_CLOSE_DELAY
• mw.popups.render.API_DELAY
• mw.popups.render.DWELL_EVENTS_MIN_INTERACTION_TIME
• mw.popups.render.cache
• mw.popups.render.renderers
• mw.popups.render.render
• mw.popups.render.reset