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