Handle clicks on descendants of links correctly in window event handler

spec/window-event-handler-spec.coffee

@@ -116,28 +116,30 @@ describe "Window", ->
       spyOn(shell, 'openExternal')
 
       link = document.createElement('a')
+      linkChild = document.createElement('span')
+      link.appendChild(linkChild)
       link.href = 'http://github.com'
       jasmine.attachToDOM(link)
-      fakeEvent = {target: link, preventDefault: (->)}
+      fakeEvent = {target: linkChild, currentTarget: link, preventDefault: (->)}
 
-      windowEventHandler.handleDocumentClick(fakeEvent)
+      windowEventHandler.handleLinkClick(fakeEvent)
       expect(shell.openExternal).toHaveBeenCalled()
       expect(shell.openExternal.argsForCall[0][0]).toBe "http://github.com"
       shell.openExternal.reset()
 
       link.href = 'https://github.com'
-      windowEventHandler.handleDocumentClick(fakeEvent)
+      windowEventHandler.handleLinkClick(fakeEvent)
       expect(shell.openExternal).toHaveBeenCalled()
       expect(shell.openExternal.argsForCall[0][0]).toBe "https://github.com"
       shell.openExternal.reset()
 
       link.href = ''
-      windowEventHandler.handleDocumentClick(fakeEvent)
+      windowEventHandler.handleLinkClick(fakeEvent)
       expect(shell.openExternal).not.toHaveBeenCalled()
       shell.openExternal.reset()
 
       link.href = '#scroll-me'
-      windowEventHandler.handleDocumentClick(fakeEvent)
+      windowEventHandler.handleLinkClick(fakeEvent)
       expect(shell.openExternal).not.toHaveBeenCalled()
 
   describe "when a form is submitted", ->

src/delegated-listener.js

@@ -0,0 +1,41 @@
+const EventKit = require('event-kit')
+
+module.exports =
+function listen (element, eventName, selector, handler) {
+  var innerHandler = function (event) {
+    if (selector) {
+      var currentTarget = event.target
+      while (true) {
+        if (currentTarget.matches && currentTarget.matches(selector)) {
+          handler({
+            type: event.type,
+            currentTarget: currentTarget,
+            target: event.target,
+            preventDefault: function () {
+              event.preventDefault()
+            },
+            originalEvent: event
+          })
+        }
+        if (currentTarget === element) break
+        currentTarget = currentTarget.parentNode
+      }
+    } else {
+      handler({
+        type: event.type,
+        currentTarget: event.currentTarget,
+        target: event.target,
+        preventDefault: function () {
+          event.preventDefault()
+        },
+        originalEvent: event
+      })
+    }
+  }
+
+  element.addEventListener(eventName, innerHandler)
+
+  return new EventKit.Disposable(function () {
+    element.removeEventListener(eventName, innerHandler)
+  })
+}

src/tooltip.js

@@ -2,6 +2,7 @@
 
 const EventKit = require('event-kit')
 const tooltipComponentsByElement = new WeakMap()
+const listen = require('./delegated-listener')
 
 // This tooltip class is derived from Bootstrap 3, but modified to not require
 // jQuery, which is an expensive dependency we want to eliminate.
@@ -452,27 +453,4 @@ function extend () {
   return target
 }
 
-function listen (element, eventName, selector, handler) {
-  var innerHandler = function (event) {
-    if (selector) {
-      var currentTarget = event.target
-      while (true) {
-        if (currentTarget.matches && currentTarget.matches(selector)) {
-          handler({type: event.type, currentTarget: currentTarget})
-        }
-        if (currentTarget === element) break
-        currentTarget = currentTarget.parentNode
-      }
-    } else {
-      handler(event)
-    }
-  }
-
-  element.addEventListener(eventName, innerHandler)
-
-  return new EventKit.Disposable(function () {
-    element.removeEventListener(eventName, innerHandler)
-  })
-}
-
 module.exports = Tooltip

src/window-event-handler.coffee

@@ -3,6 +3,7 @@ path = require 'path'
 ipc = require 'ipc'
 shell = require 'shell'
 fs = require 'fs-plus'
+listen = require './delegated-listener'
 
 # Handles low-level events related to the window.
 module.exports =
@@ -23,9 +24,9 @@ class WindowEventHandler
     @addEventListener(document, 'keydown', @handleDocumentKeydown)
     @addEventListener(document, 'drop', @handleDocumentDrop)
     @addEventListener(document, 'dragover', @handleDocumentDragover)
-    @addEventListener(document, 'click', @handleDocumentClick)
-    @addEventListener(document, 'submit', @handleDocumentSubmit)
     @addEventListener(document, 'contextmenu', @handleDocumentContextmenu)
+    @subscriptions.add listen(document, 'click', 'a', @handleLinkClick)
+    @subscriptions.add listen(document, 'submit', 'form', @handleFormSubmit)
 
     @subscriptions.add atom.commands.add window,
       'window:toggle-full-screen': @handleWindowToggleFullScreen
@@ -208,17 +209,15 @@ class WindowEventHandler
       detail = "To toggle, press the Alt key or execute the window:toggle-menu-bar command"
       atom.notifications.addInfo('Menu bar hidden', {detail})
 
-  handleDocumentClick: (event) ->
-    if (event.target.matches('a'))
-      event.preventDefault()
-      location = event.target?.getAttribute('href')
-      if location and location[0] isnt '#' and /^https?:\/\//.test(location)
-        shell.openExternal(location)
+  handleLinkClick: (event) ->
+    event.preventDefault()
+    location = event.currentTarget?.getAttribute('href')
+    if location and location[0] isnt '#' and /^https?:\/\//.test(location)
+      shell.openExternal(location)
 
-  handleDocumentSubmit: (event) ->
+  handleFormSubmit: (event) ->
     # Prevent form submits from changing the current window's URL
-    if event.target.matches('form')
-      event.preventDefault()
+    event.preventDefault()
 
   handleDocumentContextmenu: (event) ->
     event.preventDefault()