mronkko / ZoteroQuickLook

Implements QuickLook in Zotero
748 stars 82 forks source link

Firefox 60 compatibility #21

Closed dstillman closed 4 years ago

dstillman commented 4 years ago

https://groups.google.com/d/msg/zotero-dev/a1IPUJ2m_3s/jTpVxD4_BgAJ

I didn't test everything, but this fixes initialization and the main spacebar action in the items list. Only tested on macOS.

bwiernik commented 4 years ago

Thanks! Testing it on Windows, it works if I specify a value for customviewcommand pref, but if leave that blank, it fails with this error:

1575324800781   addons.manager  WARN    Exception calling callback: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "JS frame :: chrome://zoteroquicklook/content/zoteroquicklook.js :: init/< :: line 62"  data: no] Stack trace: init/<()@zoteroquicklook.js:62
safeCall()@resource://gre/modules/AddonManager.jsm:177
makeSafe/<()@resource://gre/modules/AddonManager.jsm:192

[JavaScript Error: "TypeError: this.viewerBaseArguments is null" {file: "chrome://zoteroquicklook/content/zoteroquicklook.js" line: 363}]
openQuickLook@chrome://zoteroquicklook/content/zoteroquicklook.js:363:7
handleKeyPress@chrome://zoteroquicklook/content/zoteroquicklook.js:457:5
onKey@chrome://zoteroquicklook/content/zoteroquicklook.js:437:10

The expected behavior is to look for the file localappdata\\Programs\\QuickLook\\QuickLook.exe. That seems to be failing, likely in (line 274):

else if(Zotero.isWin){
  localappdata = Components.classes["@mozilla.org/file/directory_service;1"].getService(Components.interfaces.nsIProperties).get("LocalAppData", Components.interfaces.nsIFile).path;
  this.viewerExecutable = Zotero.File.pathToFile(localappdata + "\\Programs\\QuickLook\\QuickLook.exe");
  if(this.viewerExecutable.exists() === false){
    alert("QuickLook not found. Please install QuickLook (http://pooi.moe/QuickLook/) or spesify a custom view command instead.");
  }
  this.viewerBaseArguments=[''];
}

Do you have any recommendations @dstillman?

bwiernik commented 4 years ago

One other question, how would I check if a file is downloaded and trigger download if necessary?

dstillman commented 4 years ago

OK, so, it was failing in 5.0.78 when run from an XPI (on all platforms), because Fx60 apparently no longer honors the "unpacked" flag in install.rdf for new extension installs. (It works when tested from a directory with an extension proxy file.)

I've added code that copies the Perl script to Zotero's temp directory and runs it from there. This fixes it for me on macOS, and triggers the appropriate error message on Windows.

dstillman commented 4 years ago

One other question, how would I check if a file is downloaded and trigger download if necessary?

You can't do it easily without replicating Zotero code. The relevant code is in viewAttachment() in zoteroPane.js.