8000 Refactor create‑event‑accessor.js to remove code duplication · jsdom/jsdom@e2f7639 · GitHub
[go: up one dir, main page]
Skip to content

Commit e2f7639

Browse files
ExE-Bossdomenic
ExE-Boss
authored and
domenic
committed
Refactor create‑event‑accessor.js to remove code duplication
1 parent ff69a75 commit e2f7639

File tree

2 files changed

+105
-92
lines changed

2 files changed

+105
-92
lines changed

‎lib/jsdom/browser/Window.js

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const Screen = require("../living/generated/Screen");
2727
const Storage = require("../living/generated/Storage");
2828
const Selection = require("../living/generated/Selection");
2929
const reportException = require("../living/helpers/runtime-script-errors");
30+
const { getCurrentEventHandlerValue } = require("../living/helpers/create-event-accessor.js");
3031
const { fireAnEvent } = require("../living/helpers/events");
3132
const SessionHistory = require("../living/window/SessionHistory");
3233
const { forEachMatchingSheetRuleOfElement, getResolvedValue, propertiesWithResolvedValueImplemented,
@@ -156,9 +157,12 @@ function setupWindow(windowInstance, { runScripts }) {
156157
mixin(windowInstance, GlobalEventHandlersImpl.prototype);
157158
windowInstance._initGlobalEvents();
158159

159-
// The getters are already obtained from the above mixins.
160-
// eslint-disable-next-line accessor-pairs
161160
Object.defineProperty(windowInstance, "onbeforeunload", {
161+
configurable: true,
162+
enumerable: true,
163+
get() {
164+
return idlUtils.tryWrapperForImpl(getCurrentEventHandlerValue(this, "beforeunload"));
165+
},
162166
set(V) {
163167
if (!idlUtils.isObject(V)) {
164168
V = null;
@@ -171,9 +175,12 @@ function setupWindow(windowInstance, { runScripts }) {
171175
}
172176
});
173177

174-
// The getters are already obtained from the above mixins.
175-
// eslint-disable-next-line accessor-pairs
176178
Object.defineProperty(windowInstance, "onerror", {
179+
configurable: true,
180+
enumerable: true,
181+
get() {
182+
return idlUtils.tryWrapperForImpl(getCurrentEventHandlerValue(this, "error"));
183+
},
177184
set(V) {
178185
if (!idlUtils.isObject(V)) {
179186
V = null;
@@ -187,9 +194,12 @@ function setupWindow(windowInstance, { runScripts }) {
187194
});
188195

189196
for (const event of events) {
190-
// The getters are already obtained from the above mixins.
191-
// eslint-disable-next-line accessor-pairs
192197
Object.defineProperty(windowInstance, `on${event}`, {
198+
configurable: true,
199+
enumerable: true,
200+
get() {
201+
return idlUtils.tryWrapperForImpl(getCurrentEventHandlerValue(this, event));
202+
},
193203
set(V) {
194204
if (!idlUtils.isObject(V)) {
195205
V = null;

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

Lines changed: 89 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ const OnBeforeUnloadEventHandlerNonNull = require("../generated/OnBeforeUnloadEv
77
const OnErrorEventHandlerNonNull = require("../generated/OnErrorEventHandlerNonNull.js");
88
const reportException = require("./runtime-script-errors");
99

10-
exports.appendHandler = function appendHandler(el, eventName) {
10+
exports.appendHandler = (el, eventName) => {
1111
// tryImplForWrapper() is currently required due to use in Window.js
1212
idlUtils.tryImplForWrapper(el).addEventListener(eventName, event => {
1313
// https://html.spec.whatwg.org/#the-event-handler-processing-algorithm
14-
const callback = el["on" + eventName];
14+
const callback = exports.getCurrentEventHandlerValue(el, eventName);
1515
if (callback === null) {
1616
return;
1717
}
@@ -74,103 +74,106 @@ exports.setupForSimpleEventAccessors = (prototype, events) => {
7474
}
7575
};
7676

77-
// https://html.spec.whatwg.org/#event-handler-idl-attributes
78-
exports.createEventAccessor = function createEventAccessor(obj, event) {
79-
Object.defineProperty(obj, "on" + event, {
80-
configurable: true,
81-
enumerable: true,
82-
get() {
83-
// TODO: Extract this into a helper function
84-
// https://html.spec.whatwg.org/multipage/webappapis.html#getting-the-current-value-of-the-event-handler
85-
const value = this._getEventHandlerFor(event);
86-
if (!value) {
87-
return null;
88-
}
77+
// https://html.spec.whatwg.org/multipage/webappapis.html#getting-the-current-value-of-the-event-handler
78+
exports.getCurrentEventHandlerValue = (target, event) => {
79+
const value = target._getEventHandlerFor(event);
80+
if (!value) {
81+
return null;
82+
}
8983

90-
if (value.body !== undefined) {
91-
let element;
92-
let document;
93-
if (this.constructor.name === "Window") {
94-
element = null;
95-
document = idlUtils.implForWrapper(this.document);
96-
} else {
97-
element = this;
98-
document = element.ownerDocument;
99-
}
100-
const { body } = value;
101-
102-
const formOwner = element !== null && element.form ? element.form : null;
103-
const window = this.constructor.name === "Window" && this._document ? this : document.defaultView;
104-
105-
try {
106-
// eslint-disable-next-line no-new-func
107-
Function(body); // properly error out on syntax errors
108-
// Note: this won't execute body; that would require `Function(body)()`.
109-
} catch (e) {
110-
if (window) {
111-
reportException(window, e);
112-
}
113-
this._setEventHandlerFor(event, null);
114-
return null;
115-
}
84+
if (value.body !== undefined) {
85+
let element;
86+
let document;
87+
if (target.constructor.name === "Window") {
88+
element = null;
89+
document = idlUtils.implForWrapper(target.document);
90+
} else {
91+
element = target;
92+
document = element.ownerDocument;
93+
}
94+
const { body } = value;
95+
96+
const formOwner = element !== null && element.form ? element.form : null;
97+
const window = target.constructor.name === "Window" && target._document ? target : document.defaultView;
98+
99+
try {
100+
// eslint-disable-next-line no-new-func
101+
Function(body); // properly error out on syntax errors
102+
// Note: this won't execute body; that would require `Function(body)()`.
103+
} catch (e) {
104+
if (window) {
105+
reportException(window, e);
106+
}
107+
target._setEventHandlerFor(event, null);
108+
return null;
109+
}
116110

117-
// Note: the with (window) { } is not necessary in Node, but is necessary in a browserified environment.
111+
// Note: the with (window) { } is not necessary in Node, but is necessary in a browserified environment.
118112

119-
let fn;
120-
const createFunction = document.defaultView.Function;
121-
if (event === "error" && element === null) {
122-
const wrapperBody = document ? body + `\n//# sourceURL=${document.URL}` : body;
113+
let fn;
114+
const createFunction = document.defaultView.Function;
115+
if (event === "error" && element === null) {
116+
const wrapperBody = document ? body + `\n//# sourceURL=${document.URL}` : body;
123117

124-
// eslint-disable-next-line no-new-func
125-
fn = createFunction("window", `with (window) { return function onerror(event, source, lineno, colno, error) {
118+
// eslint-disable-next-line no-new-func
119+
fn = createFunction("window", `with (window) { return function onerror(event, source, lineno, colno, error) {
126120
${wrapperBody}
127121
}; }`)(window);
128122

129-
fn = OnErrorEventHandlerNonNull.convert(fn);
130-
} else {
131-
const argNames = [];
132-
const args = [];
133-
134-
argNames.push("window");
135-
args.push(window);
136-
137-
if (element !== null) {
138-
argNames.push("document");
139-
args.push(idlUtils.wrapperForImpl(document));
140-
}
141-
if (formOwner !== null) {
142-
argNames.push("formOwner");
143-
args.push(idlUtils.wrapperForImpl(formOwner));
144-
}
145-
if (element !== null) {
146-
argNames.push("element");
147-
args.push(idlUtils.wrapperForImpl(element));
148-
}
149-
let wrapperBody = `
123+
fn = OnErrorEventHandlerNonNull.convert(fn);
124+
} else {
125+
const argNames = [];
126+
const args = [];
127+
128+
argNames.push("window");
129+
args.push(window);
130+
131+
if (element !== null) {
132+
argNames.push("document");
133+
args.push(idlUtils.wrapperForImpl(document));
134+
}
135+
if (formOwner !== null) {
136+
argNames.push("formOwner");
137+
args.push(idlUtils.wrapperForImpl(formOwner));
138+
}
139+
if (element !== null) {
140+
argNames.push("element");
141+
args.push(idlUtils.wrapperForImpl(element));
142+
}
143+
let wrapperBody = `
150144
return function on${event}(event) {
151145
${body}
152146
};`;
153-
for (let i = argNames.length - 1; i >= 0; --i) {
154-
wrapperBody = `with (${argNames[i]}) { ${wrapperBody} }`;
155-
}
156-
if (document) {
157-
wrapperBody += `\n//# sourceURL=${document.URL}`;
158-
}
159-
argNames.push(wrapperBody);
160-
fn = createFunction(...argNames)(...args);
161-
162-
if (event === "beforeunload") {
163-
fn = OnBeforeUnloadEventHandlerNonNull.convert(fn);
164-
} else {
165-
fn = EventHandlerNonNull.convert(fn);
166-
}
167-
}
147+
for (let i = argNames.length - 1; i >= 0; --i) {
148+
wrapperBody = `with (${argNames[i]}) { ${wrapperBody} }`;
149+
}
150+
if (document) {
151+
wrapperBody += `\n//# sourceURL=${document.URL}`;
152+
}
153+
argNames.push(wrapperBody);
154+
fn = createFunction(...argNames)(...args);
168155

169-
this._setEventHandlerFor(event, fn);
156+
if (event === "beforeunload") {
157+
fn = OnBeforeUnloadEventHandlerNonNull.convert(fn);
158+
} else {
159+
fn = EventHandlerNonNull.convert(fn);
170160
}
161+
}
162+
163+
target._setEventHandlerFor(event, fn);
164+
}
165+
166+
return target._getEventHandlerFor(event);
167+
};
171168

172-
// tryWrapperForImpl() is currently required due to use in Window.js
173-
return idlUtils.tryWrapperForImpl(this._getEventHandlerFor(event));
169+
// https://html.spec.whatwg.org/multipage/webappapis.html#event-handler-idl-attributes
170+
// TODO: Consider replacing this with `[ReflectEvent]`
171+
exports.createEventAccessor = (obj, event) => {
172+
Object.defineProperty(obj, "on" + event, {
173+
configurable: true,
174+
enumerable: true,
175+
get() {
176+
return exports.getCurrentEventHandlerValue(this, event);
174177
},
175178
set(val) {
176179
this._setEventHandlerFor(event, val);

0 commit comments

Comments
 (0)
0