Skip to content

Commit 5f3cbd6

Browse files
Sebmasterdomenic
Sebmaster
authored and
domenic
committed
Properly implement event handler properties/attributes
This exposes getters/setters for all the on[event] properties on the prototype, thus closing #1354. It also slots them into the correct place in the attribute list upon setting, thus closing #696. Along the way, it generally makes all the event handler processing per-spec; we pass many more web platform tests in this area.
1 parent 3eb19e7 commit 5f3cbd6

15 files changed

+356
-279
lines changed

‎lib/jsdom/browser/Window.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ const Navigator = require("../living/generated/Navigator");
2020
const reportException = require("../living/helpers/runtime-script-errors");
2121
const { contextifyWindow } = require("./documentfeatures.js");
2222

23+
const GlobalEventHandlersImpl = require("../living/nodes/GlobalEventHandlers-impl").implementation;
24+
const WindowEventHandlersImpl = require("../living/nodes/WindowEventHandlers-impl").implementation;
25+
2326
// NB: the require() must be after assigning `module.exports` because this require() is circular
2427
// TODO: this above note might not even be true anymore... figure out the cycle and document it, or clean up.
2528
module.exports = Window;
@@ -38,6 +41,8 @@ dom.Window = Window;
3841
function Window(options) {
3942
EventTarget.setup(this);
4043

44+
this._initGlobalEvents();
45+
4146
const window = this;
4247

4348
///// INTERFACES FROM THE DOM
@@ -501,6 +506,9 @@ function Window(options) {
501506
});
502507
}
503508

509+
idlUtils.mixin(Window.prototype, GlobalEventHandlersImpl.prototype);
510+
idlUtils.mixin(Window.prototype, WindowEventHandlersImpl.prototype);
511+
504512
Object.setPrototypeOf(Window, EventTarget.interface);
505513
Object.setPrototypeOf(Window.prototype, EventTarget.interface.prototype);
506514
Object.defineProperty(Window.prototype, Symbol.toStringTag, {

‎lib/jsdom/living/events/EventTarget-impl.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const reportException = require("../helpers/runtime-script-errors");
44
const domSymbolTree = require("../helpers/internal-constants").domSymbolTree;
55
const idlUtils = require("../generated/utils");
66

7+
const GlobalEventHandlers = require("../generated/GlobalEventHandlers");
78
const Event = require("../generated/Event").interface;
89

910
class EventTargetImpl {
@@ -151,11 +152,16 @@ module.exports = {
151152
};
152153

153154
function invokeInlineListeners(object, event) {
155+
// Note: for Window, object is an EventTargetImpl
154156
const wrapper = idlUtils.wrapperForImpl(object);
157+
if ((wrapper.constructor.name === "Window" && wrapper._document) || GlobalEventHandlers.isImpl(object)) {
158+
return; // elements have been upgraded to use a better path
159+
}
160+
155161
const inlineListener = getListenerForInlineEventHandler(wrapper, event.type);
156162
if (inlineListener) {
157163
// Will be falsy for windows that have closed
158-
const document = object._ownerDocument || (wrapper && (wrapper._document || wrapper._ownerDocument));
164+
const document = object._ownerDocument;
159165

160166
const runScripts = document && document._defaultView && document._defaultView._runScripts === "dangerously";
161167
if (!object.nodeName || runScripts) {
@@ -177,7 +183,7 @@ function invokeEventListeners(listeners, target, eventImpl) {
177183

178184
// workaround for events emitted on window (window-proxy)
179185
// the wrapper is the root window instance, but we only want to expose the vm proxy at all times
180-
if (wrapper._document) {
186+
if (wrapper._document && wrapper.constructor.name === "Window") {
181187
target = idlUtils.implForWrapper(wrapper._document)._defaultView;
182188
}
183189
eventImpl.currentTarget = target;

‎lib/jsdom/living/helpers/create-event-accessor.js‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
"use strict";
2+
3+
const vm = require("vm");
4+
const conversions = require("webidl-conversions");
5+
const idlUtils = require("../generated/utils");
6+
const ErrorEvent = require("../generated/ErrorEvent");
7+
const reportException = require("./runtime-script-errors");
8+
9+
exports.appendHandler = function appendHandler(el, eventName) {
10+
el.addEventListener(eventName, event => {
11+
// https://html.spec.whatwg.org/#the-event-handler-processing-algorithm
12+
event = idlUtils.implForWrapper(event);
13+
14+
const callback = el["on" + eventName];
15+
if (callback === null) {
16+
return;
17+
}
18+
19+
const specialError = ErrorEvent.isImpl(event) && event.type === "error" &&
20+
event.currentTarget.constructor.name === "Window";
21+
22+
let returnValue = null;
23+
const thisValue = idlUtils.tryWrapperForImpl(event.currentTarget);
24+
if (specialError) {
25+
returnValue = callback.call(
26+
thisValue, event.message,
27+
event.filename, event.lineno, event.colno, event.error
28+
);
29+
} else {
30+
const eventWrapper = idlUtils.wrapperForImpl(event);
31+
returnValue = callback.call(thisValue, eventWrapper);
32+
}
33+
34+
if (event.type === "beforeunload") { // TODO: we don't implement BeforeUnloadEvent so we can't brand-check here
35+
// Perform conversion which in the spec is done by the event handler return type being DOMString?
36+
returnValue = returnValue === undefined || returnValue === null ? null : conversions.DOMString(returnValue);
37+
38+
if (returnValue !== null) {
39+
event._canceledFlag = true;
40+
if (event.returnValue === "") {
41+
event.returnValue = returnValue;
42+
}
43+
}
44+
} else if (specialError) {
45+
if (returnValue === true) {
46+
event._canceledFlag = true;
47+
}
48+
} else if (returnValue === false) {
49+
event._canceledFlag = true;
50+
}
51+
});
52+
};
53+
54+
// https://html.spec.whatwg.org/#event-handler-idl-attributes
55+
exports.createEventAccessor = function createEventAccessor(obj, event) {
56+
Object.defineProperty(obj, "on" + event, {
57+
configurable: true,
58+
get() { // https://html.spec.whatwg.org/#getting-the-current-value-of-the-event-handler
59+
const value = this._getEventHandlerFor(event);
60+
if (!value) {
61+
return null;
62+
}
63+
64+
if (value.body !== undefined) {
65+
let element;
66+
let document;
67+
if (this.constructor.name === "Window") {
68+
element = null;
69+
document = idlUtils.implForWrapper(this.document);
70+
} else {
71+
element = this;
72+
document = element.ownerDocument;
73+
}
74+
const body = value.body;
75+
76+
const formOwner = element !== null && element.form ? element.form : null;
77+
const window = this.constructor.name === "Window" && this._document ? this : document.defaultView;
78+
79+
try {
80+
// eslint-disable-next-line no-new-func
81+
Function(body); // properly error out on syntax errors
82+
// Note: this won't execute body; that would require `Function(body)()`.
83+
} catch (e) {
84+
if (window) {
85+
reportException(window, e);
86+
}
87+
this._setEventHandlerFor(event, null);
88+
return null;
89+
}
90+
91+
// Note: the with (window) { } is not necessary in Node, but is necessary in a browserified environment.
92+
93+
let fn;
94+
const createFunction = vm.isContext(document._global) ? document.defaultView._globalProxy.Function : Function;
95+
if (event === "error" && element === null) {
96+
const wrapperBody = document ? body + `\n//# sourceURL=${document.URL}` : body;
97+
98+
// eslint-disable-next-line no-new-func
99+
fn = createFunction("window", `with (window) { return function onerror(event, source, lineno, colno, error) {
100+
${wrapperBody}
101+
}; }`)(window);
102+
} else {
103+
const argNames = [];
104+
const args = [];
105+
106+
argNames.push("window");
107+
args.push(window);
108+
109+
if (element !== null) {
110+
argNames.push("document");
111+
args.push(idlUtils.wrapperForImpl(document));
112+
}
113+
if (formOwner !== null) {
114+
argNames.push("formOwner");
115+
args.push(idlUtils.wrapperForImpl(formOwner));
116+
}
117+
if (element !== null) {
118+
argNames.push("element");
119+
args.push(idlUtils.wrapperForImpl(element));
120+
}
121+
let wrapperBody = `
122+
return function on${event}(event) {
123+
${body}
124+
};`;
125+
for (let i = argNames.length - 1; i >= 0; --i) {
126+
wrapperBody = `with (${argNames[i]}) { ${wrapperBody} }`;
127+
}
128+
if (document) {
129+
wrapperBody += `\n//# sourceURL=${document.URL}`;
130+
}
131+
argNames.push(wrapperBody);
132+
fn = createFunction(...argNames)(...args);
133+
}
134+
135+
this._setEventHandlerFor(event, fn);
136+
}
137+
return this._getEventHandlerFor(event);
138+
},
139+
set(val) {
140+
val = eventHandlerArgCoercion(val);
141+
this._setEventHandlerFor(event, val);
142+
}
143+
});
144+
};
145+
146+
function typeIsObject(v) {
147+
return (typeof v === "object" && v !== null) || typeof v === "function";
148+
}
149+
150+
// Implements:
151+
// [TreatNonObjectAsNull]
152+
// callback EventHandlerNonNull = any (Event event);
153+
// typedef EventHandlerNonNull? EventHandler;
154+
// Also implements the part of https://heycam.github.io/webidl/#es-invoking-callback-functions which treats
155+
// non-callable callback functions as callback functions that return undefined.
156+
// TODO: replace with webidl2js typechecking when it has sufficient callback support
157+
function eventHandlerArgCoercion(val) {
158+
if (!typeIsObject(val)) {
159+
return null;
160+
}
161+
162+
if (val === null || val === undefined) {
163+
return null;
164+
}
165+
166+
if (typeof val !== "function") {
167+
return () => {};
168+
}
169+
170+
return val;
171+
}

‎lib/jsdom/living/helpers/proxied-window-event-handlers.js‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎lib/jsdom/living/nodes/Document-impl.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const firstChildWithHTMLLocalNames = require("../helpers/traversal").firstChildW
1111
const firstDescendantWithHTMLLocalName = require("../helpers/traversal").firstDescendantWithHTMLLocalName;
1212
const whatwgURL = require("whatwg-url");
1313
const domSymbolTree = require("../helpers/internal-constants").domSymbolTree;
14+
const eventAccessors = require("../helpers/create-event-accessor");
1415
const stripAndCollapseASCIIWhitespace = require("../helpers/strings").stripAndCollapseASCIIWhitespace;
1516
const DOMException = require("../../web-idl/DOMException");
1617
const HTMLToDOM = require("../../browser/htmltodom");
@@ -22,6 +23,8 @@ const validateName = require("../helpers/validate-names").name;
2223
const validateAndExtract = require("../helpers/validate-names").validateAndExtract;
2324
const resourceLoader = require("../../browser/resource-loader");
2425

26+
const GlobalEventHandlersImpl = require("./GlobalEventHandlers-impl").implementation;
27+
2528
const clone = require("../node").clone;
2629
const generatedAttr = require("../generated/Attr");
2730
const listOfElementsWithQualifiedName = require("../node").listOfElementsWithQualifiedName;
@@ -198,6 +201,8 @@ class DocumentImpl extends NodeImpl {
198201
constructor(args, privateData) {
199202
super(args, privateData);
200203

204+
this._initGlobalEvents();
205+
201206
this._ownerDocument = this;
202207
this.nodeType = NODE_TYPE.DOCUMENT_NODE;
203208
if (!privateData.options) {
@@ -794,6 +799,8 @@ class DocumentImpl extends NodeImpl {
794799
}
795800
}
796801

802+
eventAccessors.createEventAccessor(DocumentImpl.prototype, "readystatechange");
803+
idlUtils.mixin(DocumentImpl.prototype, GlobalEventHandlersImpl.prototype);
797804
idlUtils.mixin(DocumentImpl.prototype, ParentNodeImpl.prototype);
798805

799806
DocumentImpl._removingSteps = [];

‎lib/jsdom/living/nodes/Element-impl.js‎

Lines changed: 0 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
"use strict";
2-
const vm = require("vm");
32
const nwmatcher = require("nwmatcher/src/nwmatcher-noqsa");
43
const idlUtils = require("../generated/utils");
54
const NodeImpl = require("./Node-impl").implementation;
@@ -20,7 +19,6 @@ const validateNames = require("../helpers/validate-names");
2019
const listOfElementsWithQualifiedName = require("../node").listOfElementsWithQualifiedName;
2120
const listOfElementsWithNamespaceAndLocalName = require("../node").listOfElementsWithNamespaceAndLocalName;
2221
const listOfElementsWithClassNames = require("../node").listOfElementsWithClassNames;
23-
const proxiedWindowEventHandlers = require("../helpers/proxied-window-event-handlers");
2422
const NonDocumentTypeChildNode = require("./NonDocumentTypeChildNode-impl").implementation;
2523

2624
// nwmatcher gets `document.documentElement` at creation-time, so we have to initialize lazily, since in the initial
@@ -129,75 +127,6 @@ class ElementImpl extends NodeImpl {
129127
attachId(value, this, doc);
130128
}
131129

132-
const w = this._ownerDocument._global;
133-
134-
// TODO event handlers:
135-
// The correct way to do this is lazy, and a bit more complicated; see
136-
// https://html.spec.whatwg.org/multipage/webappapis.html#event-handler-content-attributes
137-
// It would only be possible if we had proper getters/setters for every event handler, which we don't right now.
138-
if (name.length > 2 && name[0] === "o" && name[1] === "n") {
139-
// If this document does not have a window, set IDL attribute to null
140-
// step 2: https://html.spec.whatwg.org/multipage/webappapis.html#getting-the-current-value-of-the-event-handler
141-
if (value && w) {
142-
const self = proxiedWindowEventHandlers.has(name) && this._localName === "body" ? w : this;
143-
const vmOptions = { filename: this._ownerDocument.URL, displayErrors: false };
144-
145-
// The handler code probably refers to functions declared globally on the window, so we need to run it in
146-
// that context. In fact, it's worse; see
147-
// https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/bindings/core/v8/V8LazyEventListener.cpp
148-
// plus the spec, which show how multiple nested scopes are technically required. We won't implement that
149-
// until someone asks for it, though.
150-
151-
// https://html.spec.whatwg.org/multipage/webappapis.html#the-event-handler-processing-algorithm
152-
153-
if (name === "onerror" && self === w) {
154-
// https://html.spec.whatwg.org/multipage/webappapis.html#getting-the-current-value-of-the-event-handler
155-
// step 10
156-
157-
self[name] = function (event, source, lineno, colno, error) {
158-
w.__tempEventHandlerThis = this;
159-
w.__tempEventHandlerEvent = event;
160-
w.__tempEventHandlerSource = source;
161-
w.__tempEventHandlerLineno = lineno;
162-
w.__tempEventHandlerColno = colno;
163-
w.__tempEventHandlerError = error;
164-
165-
try {
166-
return vm.runInContext(`
167-
(function (event, source, lineno, colno, error) {
168-
${value}
169-
}).call(__tempEventHandlerThis, __tempEventHandlerEvent, __tempEventHandlerSource,
170-
__tempEventHandlerLineno, __tempEventHandlerColno, __tempEventHandlerError)`, w, vmOptions);
171-
} finally {
172-
delete w.__tempEventHandlerThis;
173-
delete w.__tempEventHandlerEvent;
174-
delete w.__tempEventHandlerSource;
175-
delete w.__tempEventHandlerLineno;
176-
delete w.__tempEventHandlerColno;
177-
delete w.__tempEventHandlerError;
178-
}
179-
};
180-
} else {
181-
self[name] = function (event) {
182-
w.__tempEventHandlerThis = this;
183-
w.__tempEventHandlerEvent = event;
184-
185-
try {
186-
return vm.runInContext(`
187-
(function (event) {
188-
${value}
189-
}).call(__tempEventHandlerThis, __tempEventHandlerEvent)`, w, vmOptions);
190-
} finally {
191-
delete w.__tempEventHandlerThis;
192-
delete w.__tempEventHandlerEvent;
193-
}
194-
};
195-
}
196-
} else {
197-
this[name] = null;
198-
}
199-
}
200-
201130
// update classList
202131
if (name === "class") {
203132
resetDOMTokenList(this.classList, value);

0 commit comments

Comments
 (0)