Fix event handler property generation to properly rely on runScripts

In particular, event handler properties are now generated from event handler attributes only when runScripts is set to "dangerously". This fixes #1848, by ensuring we don't generate (and then run!) event handler properties for <body>/Window when runScripts is left undefined. It also fixes #1828, by ensuring that we *do* allow the event handler property setter to work from the outside, even if runScripts is left undefined.
jsdom · May 21, 2017 · be65539 · be65539
1 parent 38c430f
commit be65539
Show file tree

Hide file tree

Showing 3 changed files with 388 additions and 6 deletions.
diff --git a/lib/jsdom/living/events/EventTarget-impl.js b/lib/jsdom/living/events/EventTarget-impl.js
@@ -155,16 +155,12 @@ function invokeInlineListeners(object, event) {
   // Note: for Window, object is an EventTargetImpl
   const wrapper = idlUtils.wrapperForImpl(object);
   if ((wrapper.constructor.name === "Window" && wrapper._document) || GlobalEventHandlers.isImpl(object)) {
-    return; // elements have been upgraded to use a better path
+    return; // elements and Window have been upgraded to use a better path; this is left for things like XHR.
   }
 
   const inlineListener = getListenerForInlineEventHandler(wrapper, event.type);
   if (inlineListener) {
-    // Will be falsy for windows that have closed
-    const document = object._ownerDocument;
-
-    const runScripts = document && document._defaultView && document._defaultView._runScripts === "dangerously";
-    if (!object.nodeName || runScripts) {
+    if (!object.nodeName) {
       invokeEventListeners([{
         callback: inlineListener,
         options: normalizeEventHandlerOptions(false, ["capture", "once"])

diff --git a/lib/jsdom/living/nodes/GlobalEventHandlers-impl.js b/lib/jsdom/living/nodes/GlobalEventHandlers-impl.js
@@ -71,6 +71,13 @@ class GlobalEventHandlersImpl {
       return;
     }
 
+    // Only translate attribute changes into properties when runScripts: "dangerously" is set.
+    // Documents without a browsing context (i.e. without a _defaultView) never run scripts.
+    const runScripts = "_runScripts" in this ? this._runScripts : (this._ownerDocument._defaultView || {})._runScripts;
+    if (runScripts !== "dangerously") {
+      return;
+    }
+
     const val = this.getAttribute(propName);
     const handler = val === null ? null : { body: val };
     this._setEventHandlerFor(event, handler);