zotero / zotero-connectors

Chrome, Firefox, Edge, and Safari extensions for Zotero
https://www.zotero.org/download/connectors
Other
530 stars 124 forks source link

`monitorDOMChanges` breakage on PDF URL in Chrome and Edge #488

Closed dstillman closed 4 months ago

dstillman commented 4 months ago

https://forums.zotero.org/discussion/115741/can-not-save-pdf-on-ieee-website-in-chrome/p1

Chrome:

(4)(+0000013): Translate: Binding sandbox to https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=9919149

(4)(+0000003): Translate: Parsing code for IEEE Xplore (92d4ed84-8d0-4d3c-941f-d4b9124cfbb, 2024-06-19 08:00:00)

(2)(+0000000): Translate: Detect using IEEE Xplore failed: 
TypeError: Cannot read properties of null (reading 'ownerDocument')

TypeError: Cannot read properties of null (reading 'ownerDocument')
    at Object.getNodeSelector (chrome-extension://ekhagklcjbdpajgpjgmbionohlpdbjgc/utilities.js:119:15)
    at UnsandboxedMutationObserver.observe (chrome-extension://ekhagklcjbdpajgpjgmbionohlpdbjgc/translateSandbox/translateSandbox.js:61:49)
    at Object.monitorDOMChanges (chrome-extension://ekhagklcjbdpajgpjgmbionohlpdbjgc/translate/translation/translate.js:751:13)
    at Object.monitorDOMChanges (chrome-extension://ekhagklcjbdpajgpjgmbionohlpdbjgc/translate/translation/sandboxManager.js:101:17)
    at detectWeb (IEEE%20Xplore:39:9)
    at Zotero.Translate.Web._detectTranslatorLoaded (chrome-extension://ekhagklcjbdpajgpjgmbionohlpdbjgc/translate/translation/translate.js:1718:47)
    at Zotero.Translate.Web._detect (chrome-extension://ekhagklcjbdpajgpjgmbionohlpdbjgc/translate/translation/translate.js:1698:22)
url => https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=9919149

Edge:

(2)(+0000000): Translate: Detect using IEEE Xplore failed: 
TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'.

TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'.
    at Object.monitorDOMChanges (chrome-extension://kamnafiakhhkladalejcocpigcjbiidm/translate/translation/translate.js:751:13)
    at Object.monitorDOMChanges (chrome-extension://kamnafiakhhkladalejcocpigcjbiidm/inject/sandboxManager.js:89:17)
    at detectWeb (eval at <anonymous> (chrome-extension://kamnafiakhhkladalejcocpigcjbiidm/inject/sandboxManager.js:63:4), <anonymous>:39:9)
    at Zotero.Translate.Web._detectTranslatorLoaded (chrome-extension://kamnafiakhhkladalejcocpigcjbiidm/translate/translation/translate.js:1718:47)
    at Zotero.Translate.Web._detect (chrome-extension://kamnafiakhhkladalejcocpigcjbiidm/translate/translation/translate.js:1698:22)
url => https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=9919149
downloadAssociatedFiles => true
automaticSnapshots => true
adomasven commented 4 months ago

Isn't this just a bug in the translator? https://github.com/zotero/translators/commit/113b77b14b446f27b04acbfa4ba3d38a99e9c25f

It's passing null to observe(), so it throws on Edge, where this code uses the MutationObserver directly and correctly, and on Chrome where MutationObserver is accessed via a proxy. The code that runs on Edge hasn't changed since MV3 introduction.

adomasven commented 4 months ago

There seems to be a bug though on Chrome and Firefox where injected scripts are not receiving messages, which is what's causing this to not work. Investigating.

EDIT: Chrome and Edge intended above.

dstillman commented 4 months ago

You mean Chrome and Edge? I guess it works in Firefox because we save PDFs a different way there.

dstillman commented 4 months ago

We should also show a clearer error and bail more gracefully if null is passed to monitorDOMChanges.

adomasven commented 4 months ago

Closed with e7540aca7cad438e2fad6defee25cab696ff67ad