dom: add onclick attribute support

+ better align attribute-based event handler behavior with other
browsers

4 files changed, 112 insertions, 53 deletions

diff --git a/src/html/catom.nim b/src/html/catom.nim
index 5a05ffab..40bfd75e 100644
--- a/src/html/catom.nim
+++ b/src/html/catom.nim
@@ -49,6 +49,7 @@ macro makeStaticAtom =
       satMultiple = "multiple"
       satName = "name"
       satNomodule = "nomodule"
+      satOnclick = "onclick"
       satOnload = "onload"
       satReferrerpolicy = "referrerpolicy"
       satRel = "rel"
diff --git a/src/html/dom.nim b/src/html/dom.nim
index aef709e9..d96877ff 100644
--- a/src/html/dom.nim
+++ b/src/html/dom.nim
@@ -2839,7 +2839,29 @@ proc loadResource(window: Window, image: HTMLImageElement) =
       )
     window.loadingResourcePromises.add(p)
 
-proc reflectAttrs(element: Element, name: CAtom, value: string) =
+proc reflectEvent(element: Element; target: EventTarget; name: StaticAtom;
+    ctype, value: string) =
+  let document = element.document
+  let ctx = document.window.jsctx
+  let urls = document.baseURL.serialize(excludepassword = true)
+  let fun = ctx.newFunction(["event"], value)
+  assert ctx != nil
+  if JS_IsException(fun):
+    let s = ctx.getExceptionStr()
+    document.window.console.log("Exception in body content attribute of",
+      urls, s)
+  else:
+    let jsTarget = ctx.toJS(target)
+    ctx.definePropertyC(jsTarget, $name, fun)
+    JS_FreeValue(ctx, jsTarget)
+    #TODO this is subtly wrong. In fact, we should not pass `fun'
+    # directly here, but a wrapper function that calls fun. Currently
+    # you can run removeEventListener with element.onclick, that should
+    # not work.
+    doAssert ctx.addEventListener(target, ctype, fun).isOk
+  JS_FreeValue(ctx, fun)
+
+proc reflectAttrs(element: Element; name: CAtom; value: string) =
   let name = element.document.toStaticAtom(name)
   template reflect_str(element: Element, n: StaticAtom, val: untyped) =
     if name == n:
@@ -2875,22 +2897,14 @@ proc reflectAttrs(element: Element, name: CAtom, value: string) =
   if name == satStyle:
     element.style_cached = newCSSStyleDeclaration(element, value)
     return
+  if name == satOnclick and element.scriptingEnabled:
+    element.reflectEvent(element, name, "click", value)
+    return
   case element.tagType
   of TAG_BODY:
     if name == satOnload and element.scriptingEnabled:
-      let document = element.document
-      let ctx = document.window.jsctx
-      let urls = document.baseURL.serialize(excludepassword = true)
-      let fun = ctx.newFunction(["event"], value)
-      assert ctx != nil
-      if JS_IsException(fun):
-        let s = ctx.getExceptionStr()
-        document.window.console.log("Exception in body content attribute of",
-          urls, s)
-      else:
-        let jsWindow = ctx.toJS(document.window)
-        ctx.definePropertyC(jsWindow, "onload", fun)
-        JS_FreeValue(ctx, jsWindow)
+      element.reflectEvent(element.document.window, name, "load", value)
+      return
   of TAG_INPUT:
     let input = HTMLInputElement(element)
     input.reflect_str satValue, value
diff --git a/src/html/event.nim b/src/html/event.nim
index 7196d151..6307cf3a 100644
--- a/src/html/event.nim
+++ b/src/html/event.nim
@@ -46,7 +46,7 @@ type
 
   EventHandler* = JSValue
 
-  EventListenerCallback = proc (event: Event): Err[JSError]
+  EventListenerCallback = JSValue
 
   EventListener* = ref object
     ctype*: string
@@ -55,7 +55,9 @@ type
     passive: Option[bool]
     once: bool
     #TODO AbortSignal
-    removed: bool
+    #TODO do we really need `removed'? maybe we could just check if
+    # callback is undefined.
+    removed*: bool
 
 jsDestructor(Event)
 jsDestructor(CustomEvent)
@@ -176,33 +178,60 @@ proc initCustomEvent(this: CustomEvent, ctype: string,
 proc newEventTarget(): EventTarget {.jsctor.} =
   return EventTarget()
 
-proc defaultPassiveValue(ctype: string, eventTarget: EventTarget): bool =
+proc defaultPassiveValue(ctype: string; eventTarget: EventTarget): bool =
   if ctype in ["touchstart", "touchmove", "wheel", "mousewheel"]:
     return true
   return eventTarget.isDefaultPassive()
 
-proc findEventListener(eventTarget: EventTarget, ctype: string,
-    callback: EventListenerCallback, capture: bool): int =
+proc findEventListener(eventTarget: EventTarget; ctype: string;
+    callback: EventListenerCallback; capture: bool): int =
   for i in 0 ..< eventTarget.eventListeners.len:
     let it = eventTarget.eventListeners[i]
     if it.ctype == ctype and it.callback == callback and it.capture == capture:
       return i
   return -1
 
+# EventListener
+proc invoke*(ctx: JSContext; listener: EventListener; event: Event):
+    JSValue =
+  #TODO make this standards compliant
+  if JS_IsNull(listener.callback):
+    return JS_UNDEFINED
+  let jsTarget = ctx.toJS(event.currentTarget)
+  var jsEvent = ctx.toJS(event)
+  if JS_IsFunction(ctx, listener.callback):
+    let ret = JS_Call(ctx, listener.callback, jsTarget, 1, addr jsEvent)
+    JS_FreeValue(ctx, jsTarget)
+    JS_FreeValue(ctx, jsEvent)
+    return ret
+  assert JS_IsObject(listener.callback)
+  let handler = JS_GetPropertyStr(ctx, listener.callback, "handleEvent")
+  if JS_IsException(handler):
+    JS_FreeValue(ctx, jsTarget)
+    JS_FreeValue(ctx, jsEvent)
+    return handler
+  let ret = JS_Call(ctx, handler, jsTarget, 1, addr jsEvent)
+  JS_FreeValue(ctx, jsTarget)
+  JS_FreeValue(ctx, jsEvent)
+  return ret
+
 # shared
-proc addAnEventListener(eventTarget: EventTarget, listener: EventListener) =
+proc addAnEventListener(target: EventTarget; listener: EventListener) =
   #TODO signals
-  if listener.callback == nil:
+  if JS_IsUndefined(listener.callback):
     return
   if listener.passive.isNone:
-    listener.passive = some(defaultPassiveValue(listener.ctype, eventTarget))
-  if eventTarget.findEventListener(listener.ctype, listener.callback,
+    listener.passive = some(defaultPassiveValue(listener.ctype, target))
+  if target.findEventListener(listener.ctype, listener.callback,
       listener.capture) == -1: # dedup
-    eventTarget.eventListeners.add(listener)
+    target.eventListeners.add(listener)
   #TODO signals
 
-proc removeAnEventListener(eventTarget: EventTarget, i: int) =
-  eventTarget.eventListeners[i].removed = true
+proc removeAnEventListener(eventTarget: EventTarget; ctx: JSContext; i: int) =
+  let listener = eventTarget.eventListeners[i]
+  listener.removed = true
+  JS_FreeValue(ctx, listener.callback)
+  listener.callback = JS_UNDEFINED
   eventTarget.eventListeners.delete(i)
 
 proc flatten(ctx: JSContext, options: JSValue): bool =
@@ -233,17 +262,21 @@ proc flattenMore(ctx: JSContext, options: JSValue):
       passive = some(x.get)
   return (capture, once, passive)
 
-proc addEventListener(ctx: JSContext, eventTarget: EventTarget, ctype: string,
-    callback: EventListenerCallback, options = JS_UNDEFINED) {.jsfunc.} =
+proc addEventListener*(ctx: JSContext; eventTarget: EventTarget; ctype: string;
+    callback: EventListenerCallback; options = JS_UNDEFINED): Err[JSError]
+    {.jsfunc.} =
+  if not JS_IsObject(callback) and not JS_IsNull(callback):
+    return errTypeError("callback is not an object")
   let (capture, once, passive) = flattenMore(ctx, options)
   let listener = EventListener(
     ctype: ctype,
     capture: capture,
     passive: passive,
     once: once,
-    callback: callback
+    callback: JS_DupValue(ctx, callback)
   )
   eventTarget.addAnEventListener(listener)
+  ok()
 
 proc removeEventListener(ctx: JSContext, eventTarget: EventTarget,
     ctype: string, callback: EventListenerCallback,
@@ -251,7 +284,7 @@ proc removeEventListener(ctx: JSContext, eventTarget: EventTarget,
   let capture = flatten(ctx, options)
   let i = eventTarget.findEventListener(ctype, callback, capture)
   if i != -1:
-    eventTarget.removeAnEventListener(i)
+    eventTarget.removeAnEventListener(ctx, i)
 
 proc addEventModule*(ctx: JSContext) =
   let eventCID = ctx.registerType(Event)
diff --git a/src/server/buffer.nim b/src/server/buffer.nim
index df1e3e9b..363f431c 100644
--- a/src/server/buffer.nim
+++ b/src/server/buffer.nim
@@ -1009,13 +1009,19 @@ proc dispatchDOMContentLoadedEvent(buffer: Buffer) =
   let document = buffer.document
   let event = newEvent(ctx, "DOMContentLoaded", document)
   var called = false
-  for el in document.eventListeners:
+  var els = document.eventListeners
+  for el in els:
+    if el.removed:
+      continue
     if el.ctype == "DOMContentLoaded":
-      let e = el.callback(event)
-      if e.isErr:
+      let e = ctx.invoke(el, event)
+      if JS_IsException(e):
         buffer.estream.write(ctx.getExceptionStr())
         buffer.estream.sflush()
+      JS_FreeValue(ctx, e)
       called = true
+      if FLAG_STOP_IMMEDIATE_PROPAGATION in event.flags:
+        break
   if called:
     buffer.do_reshape()
 
@@ -1026,43 +1032,46 @@ proc dispatchLoadEvent(buffer: Buffer) =
   let ctx = window.jsctx
   let event = newEvent(ctx, "load", window)
   var called = false
-  for el in window.eventListeners:
+  var els = window.eventListeners
+  for el in els:
+    if el.removed:
+      continue
     if el.ctype == "load":
-      let e = el.callback(event)
-      if e.isErr:
+      let e = ctx.invoke(el, event)
+      if JS_IsException(e):
         buffer.estream.write(ctx.getExceptionStr())
         buffer.estream.sflush()
+      JS_FreeValue(ctx, e)
       called = true
-  let jsWindow = toJS(ctx, window)
-  let jsonload = JS_GetPropertyStr(ctx, jsWindow, "onload")
-  var jsEvent = toJS(ctx, event)
-  if JS_IsFunction(ctx, jsonload):
-    JS_FreeValue(ctx, JS_Call(ctx, jsonload, jsWindow, 1, addr jsEvent))
-    called = true
-  JS_FreeValue(ctx, jsEvent)
-  JS_FreeValue(ctx, jsonload)
-  JS_FreeValue(ctx, jsWindow)
+      if FLAG_STOP_IMMEDIATE_PROPAGATION in event.flags:
+        break
   if called:
     buffer.do_reshape()
 
-proc dispatchEvent(buffer: Buffer, ctype: string, elem: Element): tuple[
-      called: bool,
-      canceled: bool
-    ] =
+type DispatchEventResult = tuple
+  called: bool
+  canceled: bool
+
+proc dispatchEvent(buffer: Buffer; ctype, jsName: string; elem: Element):
+    DispatchEventResult =
   var called = false
   var canceled = false
   let ctx = buffer.window.jsctx
   let event = newEvent(ctx, ctype, elem)
+  var jsEvent = ctx.toJS(event)
+  let jsNameAtom = JS_NewAtomLen(ctx, jsName, csize_t(jsName.len))
   for a in elem.branch:
     event.currentTarget = a
     var stop = false
-    for el in a.eventListeners:
+    var els = a.eventListeners
+    for el in els:
       if el.ctype == ctype:
-        let e = el.callback(event)
+        let e = ctx.invoke(el, event)
         called = true
-        if e.isErr:
+        if JS_IsException(e):
           buffer.estream.write(ctx.getExceptionStr())
           buffer.estream.sflush()
+        JS_FreeValue(ctx, e)
         if FLAG_STOP_IMMEDIATE_PROPAGATION in event.flags:
           stop = true
           break
@@ -1072,6 +1081,8 @@ proc dispatchEvent(buffer: Buffer, ctype: string, elem: Element): tuple[
           canceled = true
     if stop:
       break
+  JS_FreeValue(ctx, jsEvent)
+  JS_FreeAtom(ctx, jsNameAtom)
   return (called, canceled)
 
 proc finishLoad(buffer: Buffer): EmptyPromise =
@@ -1651,7 +1662,7 @@ proc click*(buffer: Buffer; cursorx, cursory: int): ClickResult {.proxy.} =
   let clickable = buffer.getCursorClickable(cursorx, cursory)
   if buffer.config.scripting:
     let elem = buffer.getCursorElement(cursorx, cursory)
-    (called, canceled) = buffer.dispatchEvent("click", elem)
+    (called, canceled) = buffer.dispatchEvent("click", "onclick", elem)
     if called:
       buffer.do_reshape()
   if not canceled: