Cleanup and refactoring (#2239)

* Remove unused ignoreSelectors

* Remove unused isMouseButtonPressed

* Update getWritingMode to use the immediate element if possible

* Move static functions to DocumentUtil

* Fix documentation

6 files changed, 45 insertions, 49 deletions

diff --git a/ext/js/dom/document-util.js b/ext/js/dom/document-util.js
index ed0a98e7..3934c1e2 100644
--- a/ext/js/dom/document-util.js
+++ b/ext/js/dom/document-util.js
@@ -328,16 +328,6 @@ class DocumentUtil {
         return false;
     }
 
-    static isMouseButtonPressed(mouseEvent, button) {
-        const mouseEventButton = mouseEvent.button;
-        switch (button) {
-            case 'primary': return mouseEventButton === 0;
-            case 'secondary': return mouseEventButton === 2;
-            case 'auxiliary': return mouseEventButton === 1;
-            default: return false;
-        }
-    }
-
     /**
      * Gets an array of the active modifier keys.
      * @param {KeyboardEvent|MouseEvent|TouchEvent} event The event to check.
@@ -563,6 +553,26 @@ class DocumentUtil {
         }
     }
 
+    /**
+     * Converts a value from an element to a number.
+     * @param {string} value A string representation of a number.
+     * @param {object} constraints An object which might contain `min`, `max`, and `step` fields which are used to constrain the value.
+     * @returns {number} The parsed and constrained number.
+     */
+    static convertElementValueToNumber(value, constraints) {
+        value = parseFloat(value);
+        if (!Number.isFinite(value)) { value = 0; }
+
+        let {min, max, step} = constraints;
+        min = this._convertToNumberOrNull(min);
+        max = this._convertToNumberOrNull(max);
+        step = this._convertToNumberOrNull(step);
+        if (typeof min === 'number') { value = Math.max(value, min); }
+        if (typeof max === 'number') { value = Math.min(value, max); }
+        if (typeof step === 'number' && step !== 0) { value = Math.round(value / step) * step; }
+        return value;
+    }
+
     // Private
 
     static _getActiveButtons(event, array) {
@@ -905,6 +915,16 @@ class DocumentUtil {
     static _isElementUserSelectAll(element) {
         return getComputedStyle(element).userSelect === 'all';
     }
+
+    static _convertToNumberOrNull(value) {
+        if (typeof value !== 'number') {
+            if (typeof value !== 'string' || value.length === 0) {
+                return null;
+            }
+            value = parseFloat(value);
+        }
+        return !Number.isNaN(value) ? value : null;
+    }
 }
 // eslint-disable-next-line no-underscore-dangle
 DocumentUtil._transparentColorPattern = /rgba\s*\([^)]*,\s*0(?:\.0+)?\s*\)/;
diff --git a/ext/js/dom/dom-data-binder.js b/ext/js/dom/dom-data-binder.js
index 185dc439..5835cf8c 100644
--- a/ext/js/dom/dom-data-binder.js
+++ b/ext/js/dom/dom-data-binder.js
@@ -16,14 +16,14 @@
  */
 
 /* global
+ * DocumentUtil
  * SelectorObserver
  * TaskAccumulator
  */
 
 class DOMDataBinder {
-    constructor({selector, ignoreSelectors=[], createElementMetadata, compareElementMetadata, getValues, setValues, onError=null}) {
+    constructor({selector, createElementMetadata, compareElementMetadata, getValues, setValues, onError=null}) {
         this._selector = selector;
-        this._ignoreSelectors = ignoreSelectors;
         this._createElementMetadata = createElementMetadata;
         this._compareElementMetadata = compareElementMetadata;
         this._getValues = getValues;
@@ -33,7 +33,7 @@ class DOMDataBinder {
         this._assignTasks = new TaskAccumulator(this._onBulkAssign.bind(this));
         this._selectorObserver = new SelectorObserver({
             selector,
-            ignoreSelector: (ignoreSelectors.length > 0 ? ignoreSelectors.join(',') : null),
+            ignoreSelector: null,
             onAdded: this._createObserver.bind(this),
             onRemoved: this._removeObserver.bind(this),
             onChildrenUpdated: this._onObserverChildrenUpdated.bind(this),
@@ -186,7 +186,7 @@ class DOMDataBinder {
             case 'text':
                 return `${element.value}`;
             case 'number':
-                return DOMDataBinder.convertToNumber(element.value, element);
+                return DocumentUtil.convertElementValueToNumber(element.value, element);
             case 'textarea':
             case 'select':
                 return element.value;
@@ -210,30 +210,4 @@ class DOMDataBinder {
                 return null;
         }
     }
-
-    // Utilities
-
-    static convertToNumber(value, constraints) {
-        value = parseFloat(value);
-        if (!Number.isFinite(value)) { value = 0; }
-
-        let {min, max, step} = constraints;
-        min = DOMDataBinder.convertToNumberOrNull(min);
-        max = DOMDataBinder.convertToNumberOrNull(max);
-        step = DOMDataBinder.convertToNumberOrNull(step);
-        if (typeof min === 'number') { value = Math.max(value, min); }
-        if (typeof max === 'number') { value = Math.min(value, max); }
-        if (typeof step === 'number' && step !== 0) { value = Math.round(value / step) * step; }
-        return value;
-    }
-
-    static convertToNumberOrNull(value) {
-        if (typeof value !== 'number') {
-            if (typeof value !== 'string' || value.length === 0) {
-                return null;
-            }
-            value = parseFloat(value);
-        }
-        return !Number.isNaN(value) ? value : null;
-    }
 }
diff --git a/ext/js/dom/text-source-range.js b/ext/js/dom/text-source-range.js
index a0225748..bbc22599 100644
--- a/ext/js/dom/text-source-range.js
+++ b/ext/js/dom/text-source-range.js
@@ -124,7 +124,7 @@ class TextSourceRange {
      * @param {boolean} fromEnd Whether to move the offset from the current end position (if `true`) or the start position (if `false`).
      * @param {boolean} layoutAwareScan Whether or not HTML layout information should be used to generate
      *   the string content when scanning.
-     * @returns {number} The actual number of characters (not codepoints) that were read.
+     * @returns {number} The actual number of codepoints that were read.
      */
     setEndOffset(length, fromEnd, layoutAwareScan) {
         let node;
@@ -147,7 +147,7 @@ class TextSourceRange {
      * @param {number} length The maximum number of codepoints to move by.
      * @param {boolean} layoutAwareScan Whether or not HTML layout information should be used to generate
      *   the string content when scanning.
-     * @returns {number} The actual number of characters (not codepoints) that were read.
+     * @returns {number} The actual number of codepoints that were read.
      */
     setStartOffset(length, layoutAwareScan) {
         const state = new DOMTextScanner(this._range.startContainer, this._range.startOffset, !layoutAwareScan, layoutAwareScan).seek(-length);
@@ -172,8 +172,9 @@ class TextSourceRange {
      * @returns {string} The rects.
      */
     getWritingMode() {
-        const node = this._isImposterDisconnected() ? this._imposterSourceElement : this._range.startContainer;
-        return DocumentUtil.getElementWritingMode(node !== null ? node.parentElement : null);
+        let node = this._isImposterDisconnected() ? this._imposterSourceElement : this._range.startContainer;
+        if (node !== null && node.nodeType !== Node.ELEMENT_NODE) { node = node.parentElement; }
+        return DocumentUtil.getElementWritingMode(node);
     }
 
     /**
diff --git a/ext/js/pages/settings/generic-setting-controller.js b/ext/js/pages/settings/generic-setting-controller.js
index 58f472a2..68cb1155 100644
--- a/ext/js/pages/settings/generic-setting-controller.js
+++ b/ext/js/pages/settings/generic-setting-controller.js
@@ -17,6 +17,7 @@
 
 /* globals
  * DOMDataBinder
+ * DocumentUtil
  */
 
 class GenericSettingController {
@@ -218,7 +219,7 @@ class GenericSettingController {
     _toNumber(value, data) {
         let {constraints} = data;
         if (!isObject(constraints)) { constraints = {}; }
-        return DOMDataBinder.convertToNumber(value, constraints);
+        return DocumentUtil.convertElementValueToNumber(value, constraints);
     }
 
     _toBoolean(value) {
diff --git a/ext/js/pages/settings/keyboard-shortcuts-controller.js b/ext/js/pages/settings/keyboard-shortcuts-controller.js
index 69acf275..5a3b94a6 100644
--- a/ext/js/pages/settings/keyboard-shortcuts-controller.js
+++ b/ext/js/pages/settings/keyboard-shortcuts-controller.js
@@ -16,7 +16,7 @@
  */
 
 /* global
- * DOMDataBinder
+ * DocumentUtil
  * KeyboardMouseInputField
  * ObjectPropertyAccessor
  */
@@ -322,7 +322,7 @@ class KeyboardShortcutHotkeyEntry {
         let value = this._getArgumentInputValue(node);
         switch (template) {
             case 'hotkey-argument-move-offset':
-                value = `${DOMDataBinder.convertToNumber(value, node)}`;
+                value = `${DocumentUtil.convertElementValueToNumber(value, node)}`;
                 break;
         }
         this._setArgument(value);
diff --git a/ext/js/pages/settings/nested-popups-controller.js b/ext/js/pages/settings/nested-popups-controller.js
index 8b6f02c5..cbee4e0e 100644
--- a/ext/js/pages/settings/nested-popups-controller.js
+++ b/ext/js/pages/settings/nested-popups-controller.js
@@ -16,7 +16,7 @@
  */
 
 /* global
- * DOMDataBinder
+ * DocumentUtil
  */
 
 class NestedPopupsController {
@@ -52,7 +52,7 @@ class NestedPopupsController {
 
     _onNestedPopupsCountChange(e) {
         const node = e.currentTarget;
-        const value = Math.max(1, DOMDataBinder.convertToNumber(node.value, node));
+        const value = Math.max(1, DocumentUtil.convertElementValueToNumber(node.value, node));
         this._setPopupNestingMaxDepth(value);
     }