diff options
author | toasted-nutbread <toasted-nutbread@users.noreply.github.com> | 2022-09-20 21:06:39 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-20 21:06:39 -0400 |
commit | 480869c3d1d820b344d23989d2deae64a594869e (patch) | |
tree | bc1d0c5143b71e312322e3dc851cb2c98050b8c6 | |
parent | ac373a67944a3241f96091b2bcd94f1b337a3940 (diff) |
Exclude documentElement from zoom calculation (#2227)
* Exclude documentElement from zoom calculation
* Add an option
* Refactor zoom coordinate conversion functions
* Convert zoom coordinates for text sources
* Rename variable
* Convert rect coordinate spaces
* Handle shadow DOM
-rw-r--r-- | ext/data/schemas/options-schema.json | 7 | ||||
-rw-r--r-- | ext/js/app/frontend.js | 1 | ||||
-rw-r--r-- | ext/js/app/popup.js | 58 | ||||
-rw-r--r-- | ext/js/data/options-util.js | 2 | ||||
-rw-r--r-- | ext/js/display/display.js | 2 | ||||
-rw-r--r-- | ext/js/dom/document-util.js | 87 | ||||
-rw-r--r-- | ext/js/dom/text-source-element.js | 5 | ||||
-rw-r--r-- | ext/js/dom/text-source-range.js | 4 | ||||
-rw-r--r-- | ext/js/language/text-scanner.js | 10 | ||||
-rw-r--r-- | ext/settings.html | 28 | ||||
-rw-r--r-- | test/test-document-util.js | 5 | ||||
-rw-r--r-- | test/test-options-util.js | 1 |
12 files changed, 172 insertions, 38 deletions
diff --git a/ext/data/schemas/options-schema.json b/ext/data/schemas/options-schema.json index 09398fb0..13bbbaec 100644 --- a/ext/data/schemas/options-schema.json +++ b/ext/data/schemas/options-schema.json @@ -438,7 +438,8 @@ "layoutAwareScan", "matchTypePrefix", "hidePopupOnCursorExit", - "hidePopupOnCursorExitDelay" + "hidePopupOnCursorExitDelay", + "normalizeCssZoom" ], "properties": { "inputs": { @@ -706,6 +707,10 @@ "type": "number", "minimum": 0, "default": 0 + }, + "normalizeCssZoom": { + "type": "boolean", + "default": true } } }, diff --git a/ext/js/app/frontend.js b/ext/js/app/frontend.js index f3b925c3..de3eb7fd 100644 --- a/ext/js/app/frontend.js +++ b/ext/js/app/frontend.js @@ -395,6 +395,7 @@ class Frontend { this._textScanner.setOptions({ inputs: scanningOptions.inputs, deepContentScan: scanningOptions.deepDomScan, + normalizeCssZoom: scanningOptions.normalizeCssZoom, selectText: scanningOptions.selectText, delay: scanningOptions.delay, touchInputEnabled: scanningOptions.touchInputEnabled, diff --git a/ext/js/app/popup.js b/ext/js/app/popup.js index 9ca2165d..8b62b92a 100644 --- a/ext/js/app/popup.js +++ b/ext/js/app/popup.js @@ -368,7 +368,7 @@ class Popup extends EventDispatcher { * `valid` is `false` for `PopupProxy`, since the DOM node is hosted in a different frame. */ getFrameRect() { - const {left, top, right, bottom} = this._frame.getBoundingClientRect(); + const {left, top, right, bottom} = this._getFrameBoundingClientRect(); return {left, top, right, bottom, valid: true}; } @@ -377,7 +377,7 @@ class Popup extends EventDispatcher { * @returns {Promise<{width: number, height: number, valid: boolean}>} The size and whether or not it is valid. */ async getFrameSize() { - const {width, height} = this._frame.getBoundingClientRect(); + const {width, height} = this._getFrameBoundingClientRect(); return {width, height, valid: true}; } @@ -680,12 +680,13 @@ class Popup extends EventDispatcher { * @returns {SizeRect} The calculated rectangle for where to position the popup. */ _getPosition(sourceRects, writingMode, viewport) { - const scale = this._contentScale; - const scaleRatio = this._frameSizeContentScale === null ? 1.0 : scale / this._frameSizeContentScale; - this._frameSizeContentScale = scale; + sourceRects = this._convertSourceRectsCoordinateSpace(sourceRects); + const contentScale = this._contentScale; + const scaleRatio = this._frameSizeContentScale === null ? 1.0 : contentScale / this._frameSizeContentScale; + this._frameSizeContentScale = contentScale; const frameRect = this._frame.getBoundingClientRect(); - const frameWidth = Math.max(frameRect.width * scaleRatio, this._initialWidth * scale); - const frameHeight = Math.max(frameRect.height * scaleRatio, this._initialHeight * scale); + const frameWidth = Math.max(frameRect.width * scaleRatio, this._initialWidth * contentScale); + const frameHeight = Math.max(frameRect.height * scaleRatio, this._initialHeight * contentScale); const horizontal = (writingMode === 'horizontal-tb' || this._verticalTextPosition === 'default'); let preferAfter; @@ -700,8 +701,8 @@ class Popup extends EventDispatcher { horizontalOffset = this._horizontalOffset2; verticalOffset = this._verticalOffset2; } - horizontalOffset *= scale; - verticalOffset *= scale; + horizontalOffset *= contentScale; + verticalOffset *= contentScale; let best = null; const sourceRectsLength = sourceRects.length; @@ -955,4 +956,43 @@ class Popup extends EventDispatcher { } return false; } + + /** + * Gets the bounding client rect for the frame element, with a coordinate conversion applied. + * @returns {DOMRect} The rectangle of the frame. + */ + _getFrameBoundingClientRect() { + return DocumentUtil.convertRectZoomCoordinates(this._frame.getBoundingClientRect(), this._container); + } + + /** + * Converts the coordinate space of source rectangles. + * @param {Rect[]} sourceRects The list of rectangles to convert. + * @returns {Rect[]} Either an updated list of rectangles, or `sourceRects` if no change is required. + */ + _convertSourceRectsCoordinateSpace(sourceRects) { + let scale = DocumentUtil.computeZoomScale(this._container); + if (scale === 1) { return sourceRects; } + scale = 1 / scale; + const sourceRects2 = []; + for (const rect of sourceRects) { + sourceRects2.push(this._createScaledRect(rect, scale)); + } + return sourceRects2; + } + + /** + * Creates a scaled rectangle. + * @param {Rect} rect The rectangle to scale. + * @param {number} scale The scale factor. + * @returns {Rect} A new rectangle which has been scaled. + */ + _createScaledRect(rect, scale) { + return { + left: rect.left * scale, + top: rect.top * scale, + right: rect.right * scale, + bottom: rect.bottom * scale + }; + } } diff --git a/ext/js/data/options-util.js b/ext/js/data/options-util.js index d2835adb..442007d0 100644 --- a/ext/js/data/options-util.js +++ b/ext/js/data/options-util.js @@ -980,10 +980,12 @@ class OptionsUtil { _updateVersion20(options) { // Version 20 changes: // Added anki.downloadTimeout. + // Added scanning.normalizeCssZoom. // Fixed general.popupTheme invalid default. // Fixed general.popupOuterTheme invalid default. for (const profile of options.profiles) { profile.options.anki.downloadTimeout = 0; + profile.options.scanning.normalizeCssZoom = true; const {general} = profile.options; if (general.popupTheme === 'default') { general.popupTheme = 'light'; diff --git a/ext/js/display/display.js b/ext/js/display/display.js index 8905d304..d286bd5e 100644 --- a/ext/js/display/display.js +++ b/ext/js/display/display.js @@ -367,6 +367,7 @@ class Display extends EventDispatcher { scanning: { inputs: scanningOptions.inputs, deepContentScan: scanningOptions.deepDomScan, + normalizeCssZoom: scanningOptions.normalizeCssZoom, selectText: scanningOptions.selectText, delay: scanningOptions.delay, touchInputEnabled: scanningOptions.touchInputEnabled, @@ -1532,6 +1533,7 @@ class Display extends EventDispatcher { } }], deepContentScan: scanningOptions.deepDomScan, + normalizeCssZoom: scanningOptions.normalizeCssZoom, selectText: false, delay: scanningOptions.delay, touchInputEnabled: false, diff --git a/ext/js/dom/document-util.js b/ext/js/dom/document-util.js index b974387e..41f44afe 100644 --- a/ext/js/dom/document-util.js +++ b/ext/js/dom/document-util.js @@ -24,10 +24,9 @@ class DocumentUtil { constructor() { this._transparentColorPattern = /rgba\s*\([^)]*,\s*0(?:\.0+)?\s*\)/; - this._cssZoomSupported = (typeof document.createElement('div').style.zoom === 'string'); } - getRangeFromPoint(x, y, deepContentScan) { + getRangeFromPoint(x, y, {deepContentScan, normalizeCssZoom}) { const elements = this._getElementsFromPoint(x, y, deepContentScan); let imposter = null; let imposterContainer = null; @@ -52,7 +51,7 @@ class DocumentUtil { } } - const range = this._caretRangeFromPointExt(x, y, deepContentScan ? elements : []); + const range = this._caretRangeFromPointExt(x, y, deepContentScan ? elements : [], normalizeCssZoom); if (range !== null) { if (imposter !== null) { this._setImposterStyle(imposterContainer.style, 'z-index', '-2147483646'); @@ -175,6 +174,60 @@ class DocumentUtil { }; } + /** + * Computes the scaling adjustment that is necessary for client space coordinates based on the + * CSS zoom level. + * @param {Node} node A node in the document. + * @returns {number} The scaling factor. + */ + static computeZoomScale(node) { + if (this._cssZoomSupported === null) { + this._cssZoomSupported = (typeof document.createElement('div').style.zoom === 'string'); + } + if (!this._cssZoomSupported) { return 1; } + // documentElement must be excluded because the computer style of its zoom property is inconsistent. + // * If CSS `:root{zoom:X;}` is specified, the computed zoom will always report `X`. + // * If CSS `:root{zoom:X;}` is not specified, the computed zoom report the browser's zoom level. + // Therefor, if CSS root zoom is specified as a value other than 1, the adjusted {x, y} values + // would be incorrect, which is not new behaviour. + let scale = 1; + const {ELEMENT_NODE, DOCUMENT_FRAGMENT_NODE} = Node; + const {documentElement} = document; + for (; node !== null && node !== documentElement; node = node.parentNode) { + const {nodeType} = node; + if (nodeType === DOCUMENT_FRAGMENT_NODE) { + const {host} = node; + if (typeof host !== 'undefined') { + node = host; + } + continue; + } else if (nodeType !== ELEMENT_NODE) { + continue; + } + let {zoom} = getComputedStyle(node); + if (typeof zoom !== 'string') { continue; } + zoom = Number.parseFloat(zoom); + if (!Number.isFinite(zoom) || zoom === 0) { continue; } + scale *= zoom; + } + return scale; + } + + static convertRectZoomCoordinates(rect, node) { + const scale = this.computeZoomScale(node); + return (scale === 1 ? rect : new DOMRect(rect.left * scale, rect.top * scale, rect.width * scale, rect.height * scale)); + } + + static convertMultipleRectZoomCoordinates(rects, node) { + const scale = this.computeZoomScale(node); + if (scale === 1) { return rects; } + const results = []; + for (const rect of rects) { + results.push(new DOMRect(rect.left * scale, rect.top * scale, rect.width * scale, rect.height * scale)); + } + return results; + } + static isPointInRect(x, y, rect) { return ( x >= rect.left && x < rect.right && @@ -435,7 +488,7 @@ class DocumentUtil { return e !== null ? [e] : []; } - _isPointInRange(x, y, range) { + _isPointInRange(x, y, range, normalizeCssZoom) { // Require a text node to start const {startContainer} = range; if (startContainer.nodeType !== Node.TEXT_NODE) { @@ -443,8 +496,10 @@ class DocumentUtil { } // Convert CSS zoom coordinates - if (this._cssZoomSupported) { - ({x, y} = this._convertCssZoomCoordinates(x, y, startContainer)); + if (normalizeCssZoom) { + const scale = DocumentUtil.computeZoomScale(startContainer); + x /= scale; + y /= scale; } // Scan forward @@ -583,7 +638,7 @@ class DocumentUtil { } } - _caretRangeFromPointExt(x, y, elements) { + _caretRangeFromPointExt(x, y, elements, normalizeCssZoom) { let previousStyles = null; try { let i = 0; @@ -596,7 +651,7 @@ class DocumentUtil { const startContainer = range.startContainer; if (startContinerPre !== startContainer) { - if (this._isPointInRange(x, y, range)) { + if (this._isPointInRange(x, y, range, normalizeCssZoom)) { return range; } startContinerPre = startContainer; @@ -668,18 +723,6 @@ class DocumentUtil { _isElementUserSelectAll(element) { return getComputedStyle(element).userSelect === 'all'; } - - _convertCssZoomCoordinates(x, y, node) { - const ELEMENT_NODE = Node.ELEMENT_NODE; - for (; node !== null; node = node.parentNode) { - if (node.nodeType !== ELEMENT_NODE) { continue; } - let {zoom} = getComputedStyle(node); - if (typeof zoom !== 'string') { continue; } - zoom = Number.parseFloat(zoom); - if (!Number.isFinite(zoom) || zoom === 0) { continue; } - x /= zoom; - y /= zoom; - } - return {x, y}; - } } +// eslint-disable-next-line no-underscore-dangle +DocumentUtil._cssZoomSupported = null; diff --git a/ext/js/dom/text-source-element.js b/ext/js/dom/text-source-element.js index fe3fe083..b5fc1683 100644 --- a/ext/js/dom/text-source-element.js +++ b/ext/js/dom/text-source-element.js @@ -16,6 +16,7 @@ */ /* global + * DocumentUtil * StringUtil */ @@ -95,11 +96,11 @@ class TextSourceElement { } getRect() { - return this._element.getBoundingClientRect(); + return DocumentUtil.convertRectZoomCoordinates(this._element.getBoundingClientRect(), this._element); } getRects() { - return this._element.getClientRects(); + return DocumentUtil.convertMultipleRectZoomCoordinates(this._element.getClientRects(), this._element); } getWritingMode() { diff --git a/ext/js/dom/text-source-range.js b/ext/js/dom/text-source-range.js index 5e3e814c..6c35c4cb 100644 --- a/ext/js/dom/text-source-range.js +++ b/ext/js/dom/text-source-range.js @@ -91,11 +91,11 @@ class TextSourceRange { } getRect() { - return this._range.getBoundingClientRect(); + return DocumentUtil.convertRectZoomCoordinates(this._range.getBoundingClientRect(), this._range.startContainer); } getRects() { - return this._range.getClientRects(); + return DocumentUtil.convertMultipleRectZoomCoordinates(this._range.getClientRects(), this._range.startContainer); } getWritingMode() { diff --git a/ext/js/language/text-scanner.js b/ext/js/language/text-scanner.js index 93de4dd4..3b8a8b47 100644 --- a/ext/js/language/text-scanner.js +++ b/ext/js/language/text-scanner.js @@ -54,6 +54,7 @@ class TextScanner extends EventDispatcher { this._selectionRestoreInfo = null; this._deepContentScan = false; + this._normalizeCssZoom = true; this._selectText = false; this._delay = 0; this._touchInputEnabled = false; @@ -151,6 +152,7 @@ class TextScanner extends EventDispatcher { setOptions({ inputs, deepContentScan, + normalizeCssZoom, selectText, delay, touchInputEnabled, @@ -167,6 +169,9 @@ class TextScanner extends EventDispatcher { if (typeof deepContentScan === 'boolean') { this._deepContentScan = deepContentScan; } + if (typeof normalizeCssZoom === 'boolean') { + this._normalizeCssZoom = normalizeCssZoom; + } if (typeof selectText === 'boolean') { this._selectText = selectText; } @@ -932,7 +937,10 @@ class TextScanner extends EventDispatcher { return; } - const textSource = this._documentUtil.getRangeFromPoint(x, y, this._deepContentScan); + const textSource = this._documentUtil.getRangeFromPoint(x, y, { + deepContentScan: this._deepContentScan, + normalizeCssZoom: this._normalizeCssZoom + }); try { await this._search(textSource, searchTerms, searchKanji, inputInfo); } finally { diff --git a/ext/settings.html b/ext/settings.html index 1a50a4b4..dc1405de 100644 --- a/ext/settings.html +++ b/ext/settings.html @@ -514,6 +514,34 @@ <div class="settings-item advanced-only"> <div class="settings-item-inner"> <div class="settings-item-left"> + <div class="settings-item-label">Normalize CSS zoom</div> + <div class="settings-item-description"> + Correct the pointer location on webpages where CSS <code>zoom</code> is used. + <a tabindex="0" class="more-toggle more-only" data-parent-distance="4">More…</a> + </div> + </div> + <div class="settings-item-right"> + <label class="toggle"><input type="checkbox" data-setting="scanning.normalizeCssZoom"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label> + </div> + </div> + <div class="settings-item-children more" hidden> + <p> + The non-standard CSS <a href="https://developer.mozilla.org/en-US/docs/Web/CSS/zoom" target="_blank" rel="noopener noreferrer"><code>zoom</code></a> property interferes with the normal calculation of the pointer coordinates when scanning webpages. This property is discouraged from being used and its use is rare, but some webpages may still use it. + </p> + <p> + Enabling this option, which is on by default, will take the value of this property into account when scanning webpage content. It is currently put behind an option in case there are unforeseen negative side effects. + </p> + <p> + This setting does not have any effect in Firefox, as it does not implement the <code>zoom</code> property. + </p> + <p> + <a tabindex="0" class="more-toggle" data-parent-distance="3">Less…</a> + </p> + </div> + </div> + <div class="settings-item advanced-only"> + <div class="settings-item-inner"> + <div class="settings-item-left"> <div class="settings-item-label">Wildcard scanning</div> <div class="settings-item-description"> Enable suffix wildcard when looking up scanned webpage text. diff --git a/test/test-document-util.js b/test/test-document-util.js index 334c78a5..34f4f8b3 100644 --- a/test/test-document-util.js +++ b/test/test-document-util.js @@ -167,7 +167,10 @@ async function testDocumentTextScanningFunctions(dom, {DocumentUtil, TextSourceR // Test docRangeFromPoint const documentUtil = new DocumentUtil(); - const source = documentUtil.getRangeFromPoint(0, 0, false); + const source = documentUtil.getRangeFromPoint(0, 0, { + deepContentScan: false, + normalizeCssZoom: true + }); switch (resultType) { case 'TextSourceRange': assert.strictEqual(getPrototypeOfOrNull(source), TextSourceRange.prototype); diff --git a/test/test-options-util.js b/test/test-options-util.js index af64e7dd..05af4214 100644 --- a/test/test-options-util.js +++ b/test/test-options-util.js @@ -347,6 +347,7 @@ function createProfileOptionsUpdatedTestData1() { matchTypePrefix: false, hidePopupOnCursorExit: false, hidePopupOnCursorExitDelay: 0, + normalizeCssZoom: true, preventMiddleMouse: { onWebPages: false, onPopupPages: false, |