zotero / zotero-connectors

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

Saving from Annual Reviews doesn't finish in Safari #396

Open dstillman opened 2 years ago

dstillman commented 2 years ago

Separate from #395, in the release version of the Safari connector, saving from the same Annual Reviews URL shows an empty save popup and never completes, as reported here:

https://forums.zotero.org/discussion/98608/bug-zotero-connector-and-annual-reviews

(That mentions Brave too, but it works fine for me in Chrome.)

Debug output for that shows it getting further but still not saving to Zotero:

(3)(+0000001): Translate: Beginning translation with Annual Reviews

(3)(+0000001): Translate: resolving URL http://www.annualreviews.org/action/downloadCitation

(3)(+0000000): Translate: resolved to http://www.annualreviews.org/action/downloadCitation

(3)(+0000000): Zotero.HTTP.doPost is deprecated. Use Zotero.HTTP.request

(3)(+0000012): HTTP POST http://www.annualreviews.org/action/downloadCitation

(3)(+0000008): HTTP POST http://127.0.0.1:23119/connector/getSelectedCollection

(3)(+0000043): Connector: Method getSelectedCollection succeeded

(3)(+0000178): Translate: Creating translate instance of type import in sandbox

(4)(+0000004): Translate: Binding sandbox to https://www.annualreviews.org/doi/abs/10.1146/annurev-anthro-121319-071409

(4)(+0000006): Translate: Parsing code for BibTeX (9cb70025-a888-4a29-a210-93ec52da40d4, 2021-09-10 18:25:00)

(3)(+0000013): Translate: Beginning translation with BibTeX

(3)(+0000002): Translate: Translation successful

(5)(+0000000): Translate: Running handler 0 for done

(5)(+0000000): Translate: Running handler 1 for done

(3)(+0000000): Translate: Translation successful

(5)(+0000000): Translate: Running handler 0 for done

(3)(+0003376): HTTP POST http://127.0.0.1:23119/connector/ping

(3)(+0000000): HTTP POST http://127.0.0.1:23119/connector/ping

(3)(+0000022): Connector: Method ping succeeded

(3)(+0000005): Connector: Method ping succeeded
adomasven commented 2 years ago

3 parts to this:

  1. The HTTP Post doesn't pass some Cloudflare anti-bot prevention code, so the BibTeX returned is 4 \n chars.
  2. The BibTeX import translator handles this without throwing an error, instead just returning without ever saving an item. That doesn't seem right to me.
  3. The latest translate submodule update for asyncification makes this behavior hang.

I don't think we can do much about (1). I'll address (3) once I figure out why that is. Somebody needs to say whether this makes sense for (2). @AbeJellinek ? Also it's likely reproducible on iOS.

dstillman commented 2 years ago

Why does (1) happen in Safari but not other browsers?

adomasven commented 2 years ago

BrowserExt automatically attach all available cookies to CORS bypassing HTTP requests from background pages. We only have document.cookie in Safari and that's not enough to bypass cloudflare's anti-bot detection.

AbeJellinek commented 2 years ago

(2): I think that's the correct behavior, or at least consistently the way that import translators work. They generally just don't return any items if they don't get any input. Not sure whether we have anything that relies on that, like web translators that expect a silent failure on an empty input.

Failing translation on empty/invalid input could lead to more translation errors being displayed to the user, but maybe that's good? Every so often people on the forums are confused because they try to translate a site and just see an empty progress window, usually because the endpoint we use to export metadata has changed and we're just silently failing. Might be better to show an error and fall back to a more generic translator in that case.

adomasven commented 1 year ago

Similar issue reported for the DOI translator here https://forums.zotero.org/discussion/108043/report-id-582994270-safari-on-mac-connector-does-not-save-page

Something in the translation chain should throw an error when a translator that detects for a page then does not save any items, either the translator itself or the translation framework @dstillman @zoe-translates