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: issues-unsubscr...@cordova.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

Reply via email to