From 2edd4183a0f5061999af2a4e84bea87b6e70834b Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Sat, 10 Aug 2019 13:33:00 -0400 Subject: [PATCH 1/2] Clean up docRangeFromPoint Use a cached value for checking if the browser is Firefox. Only validate and set up document.caretRangeFromPoint once. Use switch for multiple comparisons on the same value. --- ext/fg/js/document.js | 74 +++++++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 31 deletions(-) diff --git a/ext/fg/js/document.js b/ext/fg/js/document.js index f58a64fc..86396a8a 100644 --- a/ext/fg/js/document.js +++ b/ext/fg/js/document.js @@ -17,6 +17,8 @@ */ +const IS_FIREFOX = /Firefox/.test(navigator.userAgent); + function docOffsetCalc(element) { const scrollTop = window.pageYOffset || document.documentElement.scrollTop || document.body.scrollTop; const scrollLeft = window.pageXOffset || document.documentElement.scrollLeft || document.body.scrollLeft; @@ -69,43 +71,23 @@ function docRangeFromPoint(point) { const element = document.elementFromPoint(point.x, point.y); let imposter = null; if (element) { - if (element.nodeName === 'IMG' || element.nodeName === 'BUTTON') { - return new TextSourceElement(element); - } else if (element.nodeName === 'INPUT' || element.nodeName === 'TEXTAREA') { - imposter = docImposterCreate(element); + switch (element.nodeName) { + case 'IMG': + case 'BUTTON': + return new TextSourceElement(element); + case 'INPUT': + case 'TEXTAREA': + imposter = docImposterCreate(element); + break; } } - if (!document.caretRangeFromPoint) { - document.caretRangeFromPoint = (x, y) => { - const position = document.caretPositionFromPoint(x,y); - if (position && position.offsetNode && position.offsetNode.nodeType === Node.TEXT_NODE) { - const range = document.createRange(); - range.setStart(position.offsetNode, position.offset); - range.setEnd(position.offsetNode, position.offset); - return range; - } - return null; - }; - } - const range = document.caretRangeFromPoint(point.x, point.y); - if (range === null) { - return; + if (imposter !== null) { + imposter.style.zIndex = -2147483646; } - if(imposter !== null) imposter.style.zIndex = -2147483646; - - const rects = range.getClientRects(); - for (const rect of rects) { - if (point.y <= rect.bottom + 2) { - return new TextSourceRange(range); - } - } - - if (navigator.userAgent.match(/Firefox/)) { - return new TextSourceRange(range); - } + return range !== null && isPointInRange(point, range) ? new TextSourceRange(range) : null; } function docSentenceExtract(source, extent) { @@ -178,3 +160,33 @@ function docSentenceExtract(source, extent) { offset: position - startPos - padding }; } + +function isPointInRange(point, range) { + if (IS_FIREFOX) { + // Always return true on Firefox due to an issue where range.getClientRects() + // does not return a correct set of rects for characters at the beginning of a line. + return true; + } + + const y = point.y - 2; + for (const rect of range.getClientRects()) { + if (y <= rect.bottom) { + return true; + } + } + + return false; +} + +if (typeof document.caretRangeFromPoint !== 'function') { + document.caretRangeFromPoint = (x, y) => { + const position = document.caretPositionFromPoint(x, y); + if (position && position.offsetNode && position.offsetNode.nodeType === Node.TEXT_NODE) { + const range = document.createRange(); + range.setStart(position.offsetNode, position.offset); + range.setEnd(position.offsetNode, position.offset); + return range; + } + return null; + }; +} From c22f8252b94b6c310c4cae30452e1975f3097d42 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Sat, 10 Aug 2019 13:33:31 -0400 Subject: [PATCH 2/2] Handle case where docRangeFromPoint returns null --- ext/mixed/js/display.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ext/mixed/js/display.js b/ext/mixed/js/display.js index 3bb78fe1..8433c4b5 100644 --- a/ext/mixed/js/display.js +++ b/ext/mixed/js/display.js @@ -81,6 +81,9 @@ class Display { const clickedElement = $(e.target); const textSource = docRangeFromPoint({x: e.clientX, y: e.clientY}); + if (textSource === null) { + return false; + } textSource.setEndOffset(this.options.scanning.length); const {definitions, length} = await apiTermsFind(textSource.text());