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.
jsdom · Jan 21, 2024 · db0a4dc · db0a4dc
1 parent c1d7005
commit db0a4dc
Show file tree

Hide file tree

Showing 9 changed files with 150 additions and 84 deletions.
diff --git a/lib/jsdom/living/nodes/HTMLInputElement-impl.js b/lib/jsdom/living/nodes/HTMLInputElement-impl.js
@@ -748,20 +748,6 @@ class HTMLInputElementImpl extends HTMLElementImpl {
     return null;
   }
 
-  get size() {
-    if (!this.hasAttributeNS(null, "size")) {
-      return 20;
-    }
-    return parseInt(this.getAttributeNS(null, "size"));
-  }
-
-  set size(value) {
-    if (value <= 0) {
-      throw DOMException.create(this._globalObject, ["The index is not in the allowed range.", "IndexSizeError"]);
-    }
-    this.setAttributeNS(null, "size", String(value));
-  }
-
   // https://html.spec.whatwg.org/multipage/input.html#the-min-and-max-attributes
   get _minimum() {
     let min = this._defaultMinimum;

diff --git a/lib/jsdom/living/nodes/HTMLInputElement.webidl b/lib/jsdom/living/nodes/HTMLInputElement.webidl
@@ -31,7 +31,7 @@ interface HTMLInputElement : HTMLElement {
   [CEReactions, Reflect] attribute DOMString placeholder;
   [CEReactions, Reflect] attribute boolean readOnly;
   [CEReactions, Reflect] attribute boolean required;
-  [CEReactions] attribute unsigned long size;
+  [CEReactions, ReflectPositive, ReflectDefault=20] attribute unsigned long size;
   [CEReactions, ReflectURL] attribute USVString src;
   [CEReactions, Reflect] attribute DOMString step;
   [CEReactions] attribute DOMString type;

diff --git a/lib/jsdom/living/nodes/HTMLProgressElement-impl.js b/lib/jsdom/living/nodes/HTMLProgressElement-impl.js
@@ -17,17 +17,31 @@ class HTMLProgressElementImpl extends HTMLElementImpl {
   // https://html.spec.whatwg.org/multipage/form-elements.html#concept-progress-value
   get _value() {
     const valueAttr = this.getAttributeNS(null, "value");
-    const parsedValue = parseFloatingPointNumber(valueAttr);
-    if (parsedValue !== null && parsedValue > 0) {
-      return parsedValue;
+    if (valueAttr !== null) {
+      const parsedValue = parseFloatingPointNumber(valueAttr);
+      if (parsedValue !== null && parsedValue > 0) {
+        return parsedValue;
+      }
     }
     return 0;
   }
 
   // https://html.spec.whatwg.org/multipage/form-elements.html#concept-progress-current-value
   get _currentValue() {
     const value = this._value;
-    return value > this.max ? this.max : value;
+    return value > this._maximumValue ? this._maximumValue : value;
+  }
+
+  // https://html.spec.whatwg.org/multipage/form-elements.html#concept-progress-maximum
+  get _maximumValue() {
+    const maxAttr = this.getAttributeNS(null, "max");
+    if (maxAttr !== null) {
+      const parsedMax = parseFloatingPointNumber(maxAttr);
+      if (parsedMax !== null && parsedMax > 0) {
+        return parsedMax;
+      }
+    }
+    return 1.0;
   }
 
   get value() {
@@ -40,28 +54,12 @@ class HTMLProgressElementImpl extends HTMLElementImpl {
     this.setAttributeNS(null, "value", value);
   }
 
-  get max() {
-    const max = this.getAttributeNS(null, "max");
-    if (max !== null) {
-      const parsedMax = parseFloatingPointNumber(max);
-      if (parsedMax !== null && parsedMax > 0) {
-        return parsedMax;
-      }
-    }
-    return 1.0;
-  }
-  set max(value) {
-    if (value > 0) {
-      this.setAttributeNS(null, "max", value);
-    }
-  }
-
   get position() {
     if (!this._isDeterminate) {
       return -1;
     }
 
-    return this._currentValue / this.max;
+    return this._currentValue / this._maximumValue;
   }
 
   get labels() {

diff --git a/lib/jsdom/living/nodes/HTMLProgressElement.webidl b/lib/jsdom/living/nodes/HTMLProgressElement.webidl
@@ -2,7 +2,7 @@
  HTMLConstructor]
 interface HTMLProgressElement : HTMLElement {
   [CEReactions] attribute double value;
-  [CEReactions] attribute double max;
+  [CEReactions, ReflectPositive, ReflectDefault=1.0] attribute double max;
   readonly attribute double position;
   readonly attribute NodeList labels;
 };
diff --git a/lib/jsdom/living/nodes/HTMLTableCellElement.webidl b/lib/jsdom/living/nodes/HTMLTableCellElement.webidl
@@ -2,8 +2,8 @@
 [Exposed=Window,
  HTMLConstructor]
 interface HTMLTableCellElement : HTMLElement {
-  [CEReactions] attribute unsigned long colSpan;
-  [CEReactions] attribute unsigned long rowSpan;
+  [CEReactions, Reflect, ReflectRange=(1,1000), ReflectDefault=1] attribute unsigned long colSpan;
+  [CEReactions, Reflect, ReflectRange=(0,65534), ReflectDefault=1] attribute unsigned long rowSpan;
   [CEReactions, Reflect] attribute DOMString headers;
   readonly attribute long cellIndex;
 

diff --git a/lib/jsdom/living/nodes/HTMLTableColElement.webidl b/lib/jsdom/living/nodes/HTMLTableColElement.webidl
@@ -1,7 +1,7 @@
 [Exposed=Window,
  HTMLConstructor]
 interface HTMLTableColElement : HTMLElement {
-  [CEReactions, Reflect] attribute unsigned long span; // TODO: limited to only non-negative numbers greater than zero
+  [CEReactions, Reflect, ReflectRange=(1,1000), ReflectDefault=1] attribute unsigned long span;
 
   // also has obsolete members
 };

diff --git a/lib/jsdom/living/nodes/HTMLTextAreaElement-impl.js b/lib/jsdom/living/nodes/HTMLTextAreaElement-impl.js
@@ -41,7 +41,7 @@ class HTMLTextAreaElementImpl extends HTMLElementImpl {
     const apiValue = this._getAPIValue();
     const wrap = this.getAttributeNS(null, "wrap");
     return wrap === "hard" ?
-      textareaWrappingTransformation(apiValue, this.cols) :
+      textareaWrappingTransformation(apiValue, this.getAttributeNS(null, "cols") ?? 20) :
       apiValue;
   }
 
@@ -183,34 +183,6 @@ class HTMLTextAreaElementImpl extends HTMLElementImpl {
     }
   }
 
-  get cols() {
-    if (!this.hasAttributeNS(null, "cols")) {
-      return 20;
-    }
-    return parseInt(this.getAttributeNS(null, "cols"));
-  }
-
-  set cols(value) {
-    if (value <= 0) {
-      throw DOMException.create(this._globalObject, ["The index is not in the allowed range.", "IndexSizeError"]);
-    }
-    this.setAttributeNS(null, "cols", String(value));
-  }
-
-  get rows() {
-    if (!this.hasAttributeNS(null, "rows")) {
-      return 2;
-    }
-    return parseInt(this.getAttributeNS(null, "rows"));
-  }
-
-  set rows(value) {
-    if (value <= 0) {
-      throw DOMException.create(this._globalObject, ["The index is not in the allowed range.", "IndexSizeError"]);
-    }
-    this.setAttributeNS(null, "rows", String(value));
-  }
-
   _barredFromConstraintValidationSpecialization() {
     return this.hasAttributeNS(null, "readonly");
   }

diff --git a/lib/jsdom/living/nodes/HTMLTextAreaElement.webidl b/lib/jsdom/living/nodes/HTMLTextAreaElement.webidl
@@ -4,7 +4,7 @@
 interface HTMLTextAreaElement : HTMLElement {
   [CEReactions, Reflect] attribute DOMString autocomplete;
   [CEReactions, Reflect] attribute boolean autofocus;
-  [CEReactions] attribute unsigned long cols;
+  [CEReactions, ReflectPositiveWithFallback, ReflectDefault=20] attribute unsigned long cols;
   [CEReactions, Reflect] attribute DOMString dirName;
   [CEReactions, Reflect] attribute boolean disabled;
   readonly attribute HTMLFormElement? form;
@@ -15,7 +15,7 @@ interface HTMLTextAreaElement : HTMLElement {
   [CEReactions, Reflect] attribute DOMString placeholder;
   [CEReactions, Reflect] attribute boolean readOnly;
   [CEReactions, Reflect] attribute boolean required;
-  [CEReactions] attribute unsigned long rows;
+  [CEReactions, ReflectPositiveWithFallback, ReflectDefault=2] attribute unsigned long rows;
   [CEReactions, Reflect] attribute DOMString wrap;
 
   readonly attribute DOMString type;

diff --git a/scripts/webidl/convert.js b/scripts/webidl/convert.js
@@ -14,6 +14,14 @@ function isSimpleIDLType(idlType, expected) {
   return idlType.idlType === expected;
 }
 
+const recognizedReflectXAttrNames = new Set([
+  "Reflect",
+  "ReflectURL",
+  "ReflectNonNegative",
+  "ReflectPositive",
+  "ReflectPositiveWithFallback"
+]);
+
 const transformer = new Webidl2js({
   implSuffix: "-impl",
   suppressErrors: true,
@@ -39,19 +47,22 @@ const transformer = new Webidl2js({
   },
   // https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes
   processReflect(idl, implObj) {
-    const reflectAttr = idl.extAttrs.find(
-      attr => attr.name === "Reflect" || attr.name === "ReflectURL" || attr.name === "ReflectNonNegative"
-    );
+    const reflectAttr = idl.extAttrs.find(attr => recognizedReflectXAttrNames.has(attr.name));
     const attrName = reflectAttr?.rhs ? JSON.parse(reflectAttr.rhs.value) : idl.name.toLowerCase();
 
-    // TODO: [ReflectDefault] is only used for `long` right now; also use it for `unsigned long` and `double`.
     const reflectDefaultAttr = idl.extAttrs.find(attr => attr.name === "ReflectDefault");
     const reflectDefault = reflectDefaultAttr?.rhs ? JSON.parse(reflectDefaultAttr.rhs.value) : undefined;
 
+    const reflectRangeAttr = idl.extAttrs.find(attr => attr.name === "ReflectRange");
+    const reflectRange = reflectRangeAttr?.rhs ? reflectRangeAttr.rhs.value.map(v => JSON.parse(v.value)) : undefined;
+    if (reflectRange && reflectRange.length !== 2) {
+      throw new Error("Invalid [ReflectRange] value");
+    }
+
     if (reflectAttr.name === "ReflectURL") {
       // Allow DOMString also due to https://github.com/whatwg/html/issues/5241.
       if (!isSimpleIDLType(idl.idlType, "USVString") && !isSimpleIDLType(idl.idlType, "DOMString")) {
-        throw new Error("[ReflectURL] specified on non-USV/DOMString attribute");
+        throw new Error("[ReflectURL] specified on non-USVString, non-DOMString IDL attribute");
       }
       const parseURLToResultingURLRecord =
         this.addImport("../helpers/document-base-url", "parseURLToResultingURLRecord");
@@ -74,11 +85,21 @@ const transformer = new Webidl2js({
       };
     }
 
+    // For all these cases, we'll process them later, but we do the checks now.
     if (reflectAttr.name === "ReflectNonNegative") {
       if (!isSimpleIDLType(idl.idlType, "long")) {
-        throw new Error("[ReflectNonNegative] specified on non-long attribute");
+        throw new Error("[ReflectNonNegative] specified on non-long IDL attribute");
+      }
+    }
+    if (reflectAttr.name === "ReflectPositive") {
+      if (!isSimpleIDLType(idl.idlType, "unsigned long") && !isSimpleIDLType(idl.idlType, "double")) {
+        throw new Error("[ReflectPositive] specified on non-unsigned long, non-double IDL attribute");
+      }
+    }
+    if (reflectAttr.name === "ReflectPositiveWithFallback") {
+      if (!isSimpleIDLType(idl.idlType, "unsigned long")) {
+        throw new Error("[ReflectPositiveWithFallback] specified on non-unsigned long IDL attribute");
       }
-      // We'll actually do the processing in the long case, later.
     }
 
     if (isSimpleIDLType(idl.idlType, "DOMString") || isSimpleIDLType(idl.idlType, "USVString")) {
@@ -174,18 +195,107 @@ const transformer = new Webidl2js({
     if (isSimpleIDLType(idl.idlType, "unsigned long")) {
       const parseNonNegativeInteger = this.addImport("../helpers/strings", "parseNonNegativeInteger");
 
+      const minimum = reflectAttr.name === "ReflectPositive" || reflectAttr.name === "ReflectPositiveWithFallback" ?
+        1 :
+        0;
+      const maximum = 2147483647;
+      const defaultValue = reflectDefault !== undefined ? reflectDefault : minimum;
+
+      let setterPrefix = "";
+      if (reflectAttr.name === "ReflectPositive") {
+        const createDOMException = this.addImport("./DOMException", "create");
+        setterPrefix = `
+          if (V === 0) {
+            throw ${createDOMException}(
+              globalObject,
+              [\`The value \${V} cannot be set for the ${idl.name} property.\`, "IndexSizeError"]
+            );
+          }
+        `;
+      }
+
+      let get;
+      if (reflectRange) {
+        const clampedMinimum = reflectRange[0];
+        const clampedMaximum = reflectRange[1];
+        const clampedDefaultValue = reflectDefault !== undefined ? reflectDefault : clampedMinimum;
+
+        get = `
+          let value = ${implObj}._reflectGetTheContentAttribute("${attrName}");
+          if (value !== null) {
+            value = ${parseNonNegativeInteger}(value);
+            if (value !== null) {
+              if (value < ${clampedMinimum}) {
+                return ${clampedMinimum};
+              } else if (value >= ${clampedMinimum} && value <= ${clampedMaximum}) {
+                return value;
+              } else {
+                return ${clampedMaximum};
+              }
+            }
+          }
+          return ${clampedDefaultValue};
+        `;
+      } else {
+        get = `
+          let value = ${implObj}._reflectGetTheContentAttribute("${attrName}");
+          if (value !== null) {
+            value = ${parseNonNegativeInteger}(value);
+            if (value !== null && value >= ${minimum} && value <= ${maximum}) {
+              return value;
+            }
+          }
+          return ${defaultValue};
+        `;
+      }
+
+      return {
+        get,
+        set: `
+          ${setterPrefix}
+          const newValue = V <= ${maximum} && V >= ${minimum} ? V : ${defaultValue};
+          ${implObj}._reflectSetTheContentAttribute("${attrName}", String(newValue));
+        `
+      };
+    }
+
+    if (isSimpleIDLType(idl.idlType, "double")) {
+      const parseFloatingPointNumber = this.addImport("../helpers/strings", "parseFloatingPointNumber");
+      const defaultValue = reflectDefault !== undefined ? reflectDefault : 0;
+
+      if (reflectAttr.name === "ReflectPositive") {
+        return {
+          get: `
+            let value = ${implObj}._reflectGetTheContentAttribute("${attrName}");
+            if (value !== null) {
+              value = ${parseFloatingPointNumber}(value);
+              if (value !== null && value > 0) {
+                return value;
+              }
+            }
+            return ${defaultValue};
+          `,
+          set: `
+            if (V > 0) {
+              ${implObj}._reflectSetTheContentAttribute("${attrName}", String(V));
+            }
+          `
+        };
+      }
+
       return {
         get: `
           let value = ${implObj}._reflectGetTheContentAttribute("${attrName}");
-          if (value === null) {
-            return 0;
+          if (value !== null) {
+            value = ${parseFloatingPointNumber}(value);
+            if (value !== null) {
+              return value;
+            }
           }
-          value = ${parseNonNegativeInteger}(value);
-          return value !== null && value >= 0 && value <= 2147483647 ? value : 0;
+          return ${defaultValue};
         `,
         set: `
-          const n = V <= 2147483647 ? V : 0;
-          ${implObj}._reflectSetTheContentAttribute("${attrName}", String(n));
+          ${implObj}._reflectSetTheContentAttribute("${attrName}", String(V));
         `
       };
     }