DOMRect update (#1973)

* Compare using left/top rather than x/y

* Simplify

* Update Popup*.getFrameRect to return a custom structure

* Don't use x/y on DOMRect

7 files changed, 25 insertions, 30 deletions

diff --git a/ext/js/app/frontend.js b/ext/js/app/frontend.js
index 0d358639..bbc0ff85 100644
--- a/ext/js/app/frontend.js
+++ b/ext/js/app/frontend.js
@@ -543,12 +543,13 @@ class Frontend {
     }
 
     _showPopupContent(textSource, optionsContext, details=null) {
+        const {left, top, width, height} = textSource.getRect();
         this._lastShowPromise = (
             this._popup !== null ?
             this._popup.showContent(
                 {
                     optionsContext,
-                    elementRect: textSource.getRect(),
+                    elementRect: {x: left, y: top, width, height, valid: true},
                     writingMode: textSource.getWritingMode()
                 },
                 details
diff --git a/ext/js/app/popup-factory.js b/ext/js/app/popup-factory.js
index 8f0c2a6e..096d145e 100644
--- a/ext/js/app/popup-factory.js
+++ b/ext/js/app/popup-factory.js
@@ -230,7 +230,7 @@ class PopupFactory {
 
         const {elementRect} = details;
         if (typeof elementRect !== 'undefined') {
-            details.elementRect = this._convertJsonRectToDOMRect(popup, elementRect);
+            [elementRect.x, elementRect.y] = this._convertPopupPointToRootPagePoint(popup, elementRect.x, elementRect.y);
         }
 
         return await popup.showContent(details, displayDetails);
@@ -281,17 +281,14 @@ class PopupFactory {
         return popup;
     }
 
-    _convertJsonRectToDOMRect(popup, jsonRect) {
-        const [x, y] = this._convertPopupPointToRootPagePoint(popup, jsonRect.x, jsonRect.y);
-        return new DOMRect(x, y, jsonRect.width, jsonRect.height);
-    }
-
     _convertPopupPointToRootPagePoint(popup, x, y) {
-        const parent = popup.parent;
+        const {parent} = popup;
         if (parent !== null) {
             const popupRect = parent.getFrameRect();
-            x += popupRect.x;
-            y += popupRect.y;
+            if (popupRect.valid) {
+                x += popupRect.x;
+                y += popupRect.y;
+            }
         }
         return [x, y];
     }
diff --git a/ext/js/app/popup-proxy.js b/ext/js/app/popup-proxy.js
index 63ecc254..c46821d5 100644
--- a/ext/js/app/popup-proxy.js
+++ b/ext/js/app/popup-proxy.js
@@ -104,13 +104,9 @@ class PopupProxy extends EventDispatcher {
 
     async showContent(details, displayDetails) {
         const {elementRect} = details;
-        if (typeof elementRect !== 'undefined') {
-            let {x, y, width, height} = elementRect;
-            if (this._frameOffsetForwarder !== null) {
-                await this._updateFrameOffset();
-                [x, y] = this._applyFrameOffset(x, y);
-            }
-            details.elementRect = {x, y, width, height};
+        if (typeof elementRect !== 'undefined' && this._frameOffsetForwarder !== null) {
+            await this._updateFrameOffset();
+            [elementRect.x, elementRect.y] = this._applyFrameOffset(elementRect.x, elementRect.y);
         }
         return await this._invokeSafe('showContent', {id: this._id, details, displayDetails});
     }
@@ -140,7 +136,7 @@ class PopupProxy extends EventDispatcher {
     }
 
     getFrameRect() {
-        return new DOMRect(0, 0, 0, 0);
+        return {x: 0, y: 0, width: 0, height: 0, valid: false};
     }
 
     getFrameSize() {
diff --git a/ext/js/app/popup-window.js b/ext/js/app/popup-window.js
index d0826775..6f86c61e 100644
--- a/ext/js/app/popup-window.js
+++ b/ext/js/app/popup-window.js
@@ -123,7 +123,7 @@ class PopupWindow extends EventDispatcher {
     }
 
     getFrameRect() {
-        return new DOMRect(0, 0, 0, 0);
+        return {x: 0, y: 0, width: 0, height: 0, valid: false};
     }
 
     async getFrameSize() {
diff --git a/ext/js/app/popup.js b/ext/js/app/popup.js
index 8bf1d7c9..347cbb84 100644
--- a/ext/js/app/popup.js
+++ b/ext/js/app/popup.js
@@ -142,7 +142,7 @@ class Popup extends EventDispatcher {
     async containsPoint(x, y) {
         for (let popup = this; popup !== null && popup.isVisibleSync(); popup = popup.child) {
             const rect = popup.getFrameRect();
-            if (x >= rect.left && y >= rect.top && x < rect.right && y < rect.bottom) {
+            if (rect.valid && x >= rect.x && y >= rect.y && x < rect.x + rect.width && y < rect.y + rect.height) {
                 return true;
             }
         }
@@ -203,12 +203,13 @@ class Popup extends EventDispatcher {
     }
 
     getFrameRect() {
-        return this._frame.getBoundingClientRect();
+        const {left, top, width, height} = this._frame.getBoundingClientRect();
+        return {x: left, y: top, width, height, valid: true};
     }
 
     async getFrameSize() {
-        const rect = this._frame.getBoundingClientRect();
-        return {width: rect.width, height: rect.height, valid: true};
+        const {width, height} = this._frame.getBoundingClientRect();
+        return {width, height, valid: true};
     }
 
     async setFrameSize(width, height) {
@@ -535,16 +536,16 @@ class Popup extends EventDispatcher {
         const verticalOffset = optionsGeneral.popupVerticalOffset * offsetScale;
 
         const [x, w] = this._getConstrainedPosition(
-            elementRect.right - horizontalOffset,
-            elementRect.left + horizontalOffset,
+            elementRect.x + elementRect.width - horizontalOffset,
+            elementRect.x + horizontalOffset,
             width,
             viewport.left,
             viewport.right,
             true
         );
         const [y, h, below] = this._getConstrainedPositionBinary(
-            elementRect.top - verticalOffset,
-            elementRect.bottom + verticalOffset,
+            elementRect.y - verticalOffset,
+            elementRect.y + elementRect.height + verticalOffset,
             height,
             viewport.top,
             viewport.bottom,
diff --git a/ext/js/comm/frame-offset-forwarder.js b/ext/js/comm/frame-offset-forwarder.js
index 2382a9fa..dd064a64 100644
--- a/ext/js/comm/frame-offset-forwarder.js
+++ b/ext/js/comm/frame-offset-forwarder.js
@@ -63,7 +63,7 @@ class FrameOffsetForwarder {
         const frameElement = this._frameAncestryHandler.getChildFrameElement(frameId);
         if (frameElement === null) { return null; }
 
-        const {x, y, width, height} = frameElement.getBoundingClientRect();
-        return {x, y, width, height};
+        const {left, top, width, height} = frameElement.getBoundingClientRect();
+        return {x: left, y: top, width, height};
     }
 }
diff --git a/ext/js/dom/document-util.js b/ext/js/dom/document-util.js
index 0338d452..8c7adf2c 100644
--- a/ext/js/dom/document-util.js
+++ b/ext/js/dom/document-util.js
@@ -408,7 +408,7 @@ class DocumentUtil {
             this._setImposterStyle(imposterStyle, 'width', `${width}px`);
             this._setImposterStyle(imposterStyle, 'height', `${height}px`);
         }
-        if (imposterRect.x !== elementRect.x || imposterRect.y !== elementRect.y) {
+        if (imposterRect.left !== elementRect.left || imposterRect.top !== elementRect.top) {
             left += (elementRect.left - imposterRect.left);
             top += (elementRect.top - imposterRect.top);
             this._setImposterStyle(imposterStyle, 'left', `${left}px`);