Skip to content

Commit db0a4dc

Browse files
authored
Implement the remaining types of numeric reflection
This gives slight improvements to correctness on various elements, but mostly is designed to reduce hand-written code.
1 parent c1d7005 commit db0a4dc

9 files changed

Lines changed: 150 additions & 84 deletions

lib/jsdom/living/nodes/HTMLInputElement-impl.js

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -748,20 +748,6 @@ class HTMLInputElementImpl extends HTMLElementImpl {
748748
return null;
749749
}
750750

751-
get size() {
752-
if (!this.hasAttributeNS(null, "size")) {
753-
return 20;
754-
}
755-
return parseInt(this.getAttributeNS(null, "size"));
756-
}
757-
758-
set size(value) {
759-
if (value <= 0) {
760-
throw DOMException.create(this._globalObject, ["The index is not in the allowed range.", "IndexSizeError"]);
761-
}
762-
this.setAttributeNS(null, "size", String(value));
763-
}
764-
765751
// https://html.spec.whatwg.org/multipage/input.html#the-min-and-max-attributes
766752
get _minimum() {
767753
let min = this._defaultMinimum;

lib/jsdom/living/nodes/HTMLInputElement.webidl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ interface HTMLInputElement : HTMLElement {
3131
[CEReactions, Reflect] attribute DOMString placeholder;
3232
[CEReactions, Reflect] attribute boolean readOnly;
3333
[CEReactions, Reflect] attribute boolean required;
34-
[CEReactions] attribute unsigned long size;
34+
[CEReactions, ReflectPositive, ReflectDefault=20] attribute unsigned long size;
3535
[CEReactions, ReflectURL] attribute USVString src;
3636
[CEReactions, Reflect] attribute DOMString step;
3737
[CEReactions] attribute DOMString type;

lib/jsdom/living/nodes/HTMLProgressElement-impl.js

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,31 @@ class HTMLProgressElementImpl extends HTMLElementImpl {
1717
// https://html.spec.whatwg.org/multipage/form-elements.html#concept-progress-value
1818
get _value() {
1919
const valueAttr = this.getAttributeNS(null, "value");
20-
const parsedValue = parseFloatingPointNumber(valueAttr);
21-
if (parsedValue !== null && parsedValue > 0) {
22-
return parsedValue;
20+
if (valueAttr !== null) {
21+
const parsedValue = parseFloatingPointNumber(valueAttr);
22+
if (parsedValue !== null && parsedValue > 0) {
23+
return parsedValue;
24+
}
2325
}
2426
return 0;
2527
}
2628

2729
// https://html.spec.whatwg.org/multipage/form-elements.html#concept-progress-current-value
2830
get _currentValue() {
2931
const value = this._value;
30-
return value > this.max ? this.max : value;
32+
return value > this._maximumValue ? this._maximumValue : value;
33+
}
34+
35+
// https://html.spec.whatwg.org/multipage/form-elements.html#concept-progress-maximum
36+
get _maximumValue() {
37+
const maxAttr = this.getAttributeNS(null, "max");
38+
if (maxAttr !== null) {
39+
const parsedMax = parseFloatingPointNumber(maxAttr);
40+
if (parsedMax !== null && parsedMax > 0) {
41+
return parsedMax;
42+
}
43+
}
44+
return 1.0;
3145
}
3246

3347
get value() {
@@ -40,28 +54,12 @@ class HTMLProgressElementImpl extends HTMLElementImpl {
4054
this.setAttributeNS(null, "value", value);
4155
}
4256

43-
get max() {
44-
const max = this.getAttributeNS(null, "max");
45-
if (max !== null) {
46-
const parsedMax = parseFloatingPointNumber(max);
47-
if (parsedMax !== null && parsedMax > 0) {
48-
return parsedMax;
49-
}
50-
}
51-
return 1.0;
52-
}
53-
set max(value) {
54-
if (value > 0) {
55-
this.setAttributeNS(null, "max", value);
56-
}
57-
}
58-
5957
get position() {
6058
if (!this._isDeterminate) {
6159
return -1;
6260
}
6361

64-
return this._currentValue / this.max;
62+
return this._currentValue / this._maximumValue;
6563
}
6664

6765
get labels() {

lib/jsdom/living/nodes/HTMLProgressElement.webidl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
HTMLConstructor]
33
interface HTMLProgressElement : HTMLElement {
44
[CEReactions] attribute double value;
5-
[CEReactions] attribute double max;
5+
[CEReactions, ReflectPositive, ReflectDefault=1.0] attribute double max;
66
readonly attribute double position;
77
readonly attribute NodeList labels;
88
};

lib/jsdom/living/nodes/HTMLTableCellElement.webidl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
[Exposed=Window,
33
HTMLConstructor]
44
interface HTMLTableCellElement : HTMLElement {
5-
[CEReactions] attribute unsigned long colSpan;
6-
[CEReactions] attribute unsigned long rowSpan;
5+
[CEReactions, Reflect, ReflectRange=(1,1000), ReflectDefault=1] attribute unsigned long colSpan;
6+
[CEReactions, Reflect, ReflectRange=(0,65534), ReflectDefault=1] attribute unsigned long rowSpan;
77
[CEReactions, Reflect] attribute DOMString headers;
88
readonly attribute long cellIndex;
99

lib/jsdom/living/nodes/HTMLTableColElement.webidl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[Exposed=Window,
22
HTMLConstructor]
33
interface HTMLTableColElement : HTMLElement {
4-
[CEReactions, Reflect] attribute unsigned long span; // TODO: limited to only non-negative numbers greater than zero
4+
[CEReactions, Reflect, ReflectRange=(1,1000), ReflectDefault=1] attribute unsigned long span;
55

66
// also has obsolete members
77
};

lib/jsdom/living/nodes/HTMLTextAreaElement-impl.js

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class HTMLTextAreaElementImpl extends HTMLElementImpl {
4141
const apiValue = this._getAPIValue();
4242
const wrap = this.getAttributeNS(null, "wrap");
4343
return wrap === "hard" ?
44-
textareaWrappingTransformation(apiValue, this.cols) :
44+
textareaWrappingTransformation(apiValue, this.getAttributeNS(null, "cols") ?? 20) :
4545
apiValue;
4646
}
4747

@@ -183,34 +183,6 @@ class HTMLTextAreaElementImpl extends HTMLElementImpl {
183183
}
184184
}
185185

186-
get cols() {
187-
if (!this.hasAttributeNS(null, "cols")) {
188-
return 20;
189-
}
190-
return parseInt(this.getAttributeNS(null, "cols"));
191-
}
192-
193-
set cols(value) {
194-
if (value <= 0) {
195-
throw DOMException.create(this._globalObject, ["The index is not in the allowed range.", "IndexSizeError"]);
196-
}
197-
this.setAttributeNS(null, "cols", String(value));
198-
}
199-
200-
get rows() {
201-
if (!this.hasAttributeNS(null, "rows")) {
202-
return 2;
203-
}
204-
return parseInt(this.getAttributeNS(null, "rows"));
205-
}
206-
207-
set rows(value) {
208-
if (value <= 0) {
209-
throw DOMException.create(this._globalObject, ["The index is not in the allowed range.", "IndexSizeError"]);
210-
}
211-
this.setAttributeNS(null, "rows", String(value));
212-
}
213-
214186
_barredFromConstraintValidationSpecialization() {
215187
return this.hasAttributeNS(null, "readonly");
216188
}

lib/jsdom/living/nodes/HTMLTextAreaElement.webidl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
interface HTMLTextAreaElement : HTMLElement {
55
[CEReactions, Reflect] attribute DOMString autocomplete;
66
[CEReactions, Reflect] attribute boolean autofocus;
7-
[CEReactions] attribute unsigned long cols;
7+
[CEReactions, ReflectPositiveWithFallback, ReflectDefault=20] attribute unsigned long cols;
88
[CEReactions, Reflect] attribute DOMString dirName;
99
[CEReactions, Reflect] attribute boolean disabled;
1010
readonly attribute HTMLFormElement? form;
@@ -15,7 +15,7 @@ interface HTMLTextAreaElement : HTMLElement {
1515
[CEReactions, Reflect] attribute DOMString placeholder;
1616
[CEReactions, Reflect] attribute boolean readOnly;
1717
[CEReactions, Reflect] attribute boolean required;
18-
[CEReactions] attribute unsigned long rows;
18+
[CEReactions, ReflectPositiveWithFallback, ReflectDefault=2] attribute unsigned long rows;
1919
[CEReactions, Reflect] attribute DOMString wrap;
2020

2121
readonly attribute DOMString type;

scripts/webidl/convert.js

Lines changed: 123 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ function isSimpleIDLType(idlType, expected) {
1414
return idlType.idlType === expected;
1515
}
1616

17+
const recognizedReflectXAttrNames = new Set([
18+
"Reflect",
19+
"ReflectURL",
20+
"ReflectNonNegative",
21+
"ReflectPositive",
22+
"ReflectPositiveWithFallback"
23+
]);
24+
1725
const transformer = new Webidl2js({
1826
implSuffix: "-impl",
1927
suppressErrors: true,
@@ -39,19 +47,22 @@ const transformer = new Webidl2js({
3947
},
4048
// https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes
4149
processReflect(idl, implObj) {
42-
const reflectAttr = idl.extAttrs.find(
43-
attr => attr.name === "Reflect" || attr.name === "ReflectURL" || attr.name === "ReflectNonNegative"
44-
);
50+
const reflectAttr = idl.extAttrs.find(attr => recognizedReflectXAttrNames.has(attr.name));
4551
const attrName = reflectAttr?.rhs ? JSON.parse(reflectAttr.rhs.value) : idl.name.toLowerCase();
4652

47-
// TODO: [ReflectDefault] is only used for `long` right now; also use it for `unsigned long` and `double`.
4853
const reflectDefaultAttr = idl.extAttrs.find(attr => attr.name === "ReflectDefault");
4954
const reflectDefault = reflectDefaultAttr?.rhs ? JSON.parse(reflectDefaultAttr.rhs.value) : undefined;
5055

56+
const reflectRangeAttr = idl.extAttrs.find(attr => attr.name === "ReflectRange");
57+
const reflectRange = reflectRangeAttr?.rhs ? reflectRangeAttr.rhs.value.map(v => JSON.parse(v.value)) : undefined;
58+
if (reflectRange && reflectRange.length !== 2) {
59+
throw new Error("Invalid [ReflectRange] value");
60+
}
61+
5162
if (reflectAttr.name === "ReflectURL") {
5263
// Allow DOMString also due to https://github.com/whatwg/html/issues/5241.
5364
if (!isSimpleIDLType(idl.idlType, "USVString") && !isSimpleIDLType(idl.idlType, "DOMString")) {
54-
throw new Error("[ReflectURL] specified on non-USV/DOMString attribute");
65+
throw new Error("[ReflectURL] specified on non-USVString, non-DOMString IDL attribute");
5566
}
5667
const parseURLToResultingURLRecord =
5768
this.addImport("../helpers/document-base-url", "parseURLToResultingURLRecord");
@@ -74,11 +85,21 @@ const transformer = new Webidl2js({
7485
};
7586
}
7687

88+
// For all these cases, we'll process them later, but we do the checks now.
7789
if (reflectAttr.name === "ReflectNonNegative") {
7890
if (!isSimpleIDLType(idl.idlType, "long")) {
79-
throw new Error("[ReflectNonNegative] specified on non-long attribute");
91+
throw new Error("[ReflectNonNegative] specified on non-long IDL attribute");
92+
}
93+
}
94+
if (reflectAttr.name === "ReflectPositive") {
95+
if (!isSimpleIDLType(idl.idlType, "unsigned long") && !isSimpleIDLType(idl.idlType, "double")) {
96+
throw new Error("[ReflectPositive] specified on non-unsigned long, non-double IDL attribute");
97+
}
98+
}
99+
if (reflectAttr.name === "ReflectPositiveWithFallback") {
100+
if (!isSimpleIDLType(idl.idlType, "unsigned long")) {
101+
throw new Error("[ReflectPositiveWithFallback] specified on non-unsigned long IDL attribute");
80102
}
81-
// We'll actually do the processing in the long case, later.
82103
}
83104

84105
if (isSimpleIDLType(idl.idlType, "DOMString") || isSimpleIDLType(idl.idlType, "USVString")) {
@@ -174,18 +195,107 @@ const transformer = new Webidl2js({
174195
if (isSimpleIDLType(idl.idlType, "unsigned long")) {
175196
const parseNonNegativeInteger = this.addImport("../helpers/strings", "parseNonNegativeInteger");
176197

198+
const minimum = reflectAttr.name === "ReflectPositive" || reflectAttr.name === "ReflectPositiveWithFallback" ?
199+
1 :
200+
0;
201+
const maximum = 2147483647;
202+
const defaultValue = reflectDefault !== undefined ? reflectDefault : minimum;
203+
204+
let setterPrefix = "";
205+
if (reflectAttr.name === "ReflectPositive") {
206+
const createDOMException = this.addImport("./DOMException", "create");
207+
setterPrefix = `
208+
if (V === 0) {
209+
throw ${createDOMException}(
210+
globalObject,
211+
[\`The value \${V} cannot be set for the ${idl.name} property.\`, "IndexSizeError"]
212+
);
213+
}
214+
`;
215+
}
216+
217+
let get;
218+
if (reflectRange) {
219+
const clampedMinimum = reflectRange[0];
220+
const clampedMaximum = reflectRange[1];
221+
const clampedDefaultValue = reflectDefault !== undefined ? reflectDefault : clampedMinimum;
222+
223+
get = `
224+
let value = ${implObj}._reflectGetTheContentAttribute("${attrName}");
225+
if (value !== null) {
226+
value = ${parseNonNegativeInteger}(value);
227+
if (value !== null) {
228+
if (value < ${clampedMinimum}) {
229+
return ${clampedMinimum};
230+
} else if (value >= ${clampedMinimum} && value <= ${clampedMaximum}) {
231+
return value;
232+
} else {
233+
return ${clampedMaximum};
234+
}
235+
}
236+
}
237+
return ${clampedDefaultValue};
238+
`;
239+
} else {
240+
get = `
241+
let value = ${implObj}._reflectGetTheContentAttribute("${attrName}");
242+
if (value !== null) {
243+
value = ${parseNonNegativeInteger}(value);
244+
if (value !== null && value >= ${minimum} && value <= ${maximum}) {
245+
return value;
246+
}
247+
}
248+
return ${defaultValue};
249+
`;
250+
}
251+
252+
return {
253+
get,
254+
set: `
255+
${setterPrefix}
256+
const newValue = V <= ${maximum} && V >= ${minimum} ? V : ${defaultValue};
257+
${implObj}._reflectSetTheContentAttribute("${attrName}", String(newValue));
258+
`
259+
};
260+
}
261+
262+
if (isSimpleIDLType(idl.idlType, "double")) {
263+
const parseFloatingPointNumber = this.addImport("../helpers/strings", "parseFloatingPointNumber");
264+
const defaultValue = reflectDefault !== undefined ? reflectDefault : 0;
265+
266+
if (reflectAttr.name === "ReflectPositive") {
267+
return {
268+
get: `
269+
let value = ${implObj}._reflectGetTheContentAttribute("${attrName}");
270+
if (value !== null) {
271+
value = ${parseFloatingPointNumber}(value);
272+
if (value !== null && value > 0) {
273+
return value;
274+
}
275+
}
276+
return ${defaultValue};
277+
`,
278+
set: `
279+
if (V > 0) {
280+
${implObj}._reflectSetTheContentAttribute("${attrName}", String(V));
281+
}
282+
`
283+
};
284+
}
285+
177286
return {
178287
get: `
179288
let value = ${implObj}._reflectGetTheContentAttribute("${attrName}");
180-
if (value === null) {
181-
return 0;
289+
if (value !== null) {
290+
value = ${parseFloatingPointNumber}(value);
291+
if (value !== null) {
292+
return value;
293+
}
182294
}
183-
value = ${parseNonNegativeInteger}(value);
184-
return value !== null && value >= 0 && value <= 2147483647 ? value : 0;
295+
return ${defaultValue};
185296
`,
186297
set: `
187-
const n = V <= 2147483647 ? V : 0;
188-
${implObj}._reflectSetTheContentAttribute("${attrName}", String(n));
298+
${implObj}._reflectSetTheContentAttribute("${attrName}", String(V));
189299
`
190300
};
191301
}

0 commit comments

Comments
 (0)