breautek commented on code in PR #276:
URL: https://github.com/apache/cordova-js/pull/276#discussion_r1706269839
##########
src/cordova.js:
##########
@@ -38,48 +38,102 @@ var m_window_addEventListener = window.addEventListener;
var m_window_removeEventListener = window.removeEventListener;
/**
- * Houses custom event handlers to intercept on document + window event
listeners.
+ * Globally accessible event handlers.
*/
-var documentEventHandlers = {};
-var windowEventHandlers = {};
-
-document.addEventListener = function (evt, handler, capture) {
- var e = evt.toLowerCase();
- if (typeof documentEventHandlers[e] !== 'undefined') {
- documentEventHandlers[e].subscribe(handler);
- } else {
- m_document_addEventListener.call(document, evt, handler, capture);
- }
-};
+var documentEventHandlers;
+var windowEventHandlers;
Review Comment:
As per the original report, you claimed these must not be globally
accessible so they should be declared in the IIFE function, so that they will
be out of scope of any external code.
Honestly I'm still not sure if this will be breaking or if there is existing
cordova code that is external that depends on these global variables. I'll need
to consult with other members who are more familiar with this part of the
codebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
