search

jlegewie / zotfile

Zotero plugin to manage your attachments: automatically rename, move, and attach PDFs (or other files) to Zotero items, sync PDFs from your Zotero library to your (mobile) PDF reader (e.g. an iPad, Android tablet, etc.), and extract PDF annotations.
4.05k stars 281 forks source link

Possible interference between Better BibTeX and Zotfile #220

Open retorquere opened 8 years ago

retorquere commented 8 years ago

I have a user report on Better BibTeX that seems to indicate a possible interference between BBT and Zotfile. In the users' logfile, I see this:

[JavaScript Error: "TypeError: Zotero.ZotFile.pdfAnnotations is undefined" {file: "chrome://zotfile/content/zotfile.js" line: 115}]
Zotero.ZotFile.init/<@chrome://zotfile/content/zotfile.js:115:1
Zotero.ZotFile.futureRun/<.run@chrome://zotfile/content/zotfile.js:754:49
getContentsFromURL@chrome://zotero/content/xpcom/file.js:214:3
Zotero.BetterBibTeX.getContentsFromURL@chrome://zotero-better-bibtex/content/zotero-better-bibtex.js:1727:12
Zotero.BetterBibTeX.load@chrome://zotero-better-bibtex/content/zotero-better-bibtex.js:1738:12
Zotero.BetterBibTeX.loadTranslators@chrome://zotero-better-bibtex/content/zotero-better-bibtex.js:1397:5

I can explain everything down to getContentsFromURL, which is where BBT is trying to load one of its bundled translators, but I don't think you are monkey-patching getContentsFromURL (at least I couldn't find it if you do), so I can't readily explain why it triggers a call to Zotero.ZotFile.futureRun, never mind why that in turn can run at all before the point where Zotero.ZotFile.pdfAnnotations is defined.

Any insights would be very much appreciated, I wouldn't want to conflict with Zotfile.

jlegewie commented 8 years ago

Not really sure why this is happens. I am not monkey-patching getContentsFromURL. Can you ask the user whether the same problem still occurs in 4.2.6? The version is under review at mozilla but you can already get it here: https://addons.mozilla.org/en-US/firefox/addon/zotfile/versions At least the TypeError: Zotero.ZotFile.pdfAnnotations should be fixed.

retorquere commented 8 years ago

Ah, the wonderful Mozilla review process. Not a fan myself.

I've asked @melsophos (who reported the issue) to upgrade and test, but I'm thinking it's just a race condition; FF pre-e10n is single threaded, and your futureRun code simply interrupted some arbitry code, errored, and the rest is collateral damage. BBT just happened to get caught in it. @melsophos saw the problem go away after disabling and re-enabling Zotfile, which strengthens my idea of the race condition. The error is still in the logs though, whether BBT was interrupted or not, so I've requested a new log with 4.2.6 installed.

retorquere commented 8 years ago

This error is gone after installing 4.2.6, so I can consider this issue closed. The issue in BBT that I thought was related still exists, so I'm looking into that further.

retorquere commented 8 years ago

I'm currently trying to figure out whether this error is in some way related to BBT:

TypeError: att.getFile(...).exists is not a function" {file: "chrome://zotfile/content/zotfile.js" line: 3433

I can't trigger it myself, but I see it sometimes in logs from BBT users. The only reason I can think would be related is that I fiddle with nextItem in the translator, but this call doesn't seem to be part of the export process, and the resulting objects don't have getFile() at all, so it shouldn't error out with exists is not a function but getFile is not a function

jlegewie commented 8 years ago

I think this is just a zotfile bug. Maybe BBT triggers events that lead to the zotfile bug. att.getFile(...).exists is not a function has been an elusive error message. I am not going to look into it for the current version though. I am doing some (slow) work in the background (not yet on github) to prepare zotfile for Zotero 5.0. The transition changes so many parts of the code and I think it makes more sense to look into it if it occurs with Zotero 5 and zotfile 5 again.

retorquere commented 8 years ago

Would it be possible to test for functionness of exists in the interim? It could remove the risk of collateral errors.

jlegewie commented 8 years ago

Can you try with the current version on github (master branch)? It's possible that I fixed it a couple of weeks ago and if not it would be good to have the line number for the current version.

retorquere commented 8 years ago

I haven't been able to trigger the problem myself, I just see it sometimes in logs that are submitted to me.

phretor commented 8 years ago

I'm having this problem with ZotFile 4.2.6 and BetterBibLaTeX 1.6.55 and 1.6.56 and this is the error that I'm getting when I'm trying to send files to the tablet:

TypeError: tablet_file.exists is not a function (chrome://zotfile/content/zotfile.js, 2782)

and this one when I'm trying to fetch from the tablet:

Error: TypeError: f.exists is not a function

I've tried to disable and remove the BBL extension and I'm getting the same errors.