search

UB-Mannheim / zotero-ocr

Zotero Plugin for OCR
GNU Affero General Public License v3.0
552 stars 40 forks source link

Does this plugin work with Zotero 6 and 7 #73

Closed denismaier closed 5 months ago

denismaier commented 6 months ago

Hi, I've noticed there's some code in this repo that makes me guess this plugin should work with Zotero 6 and 7. However, other things differ quite a bit from the best practices outlined in the sample plugin. (E.g., here you use doc.createXULElement('menuitem'); instead of doc.createElementNS(XUL_NS, 'menuitem');.

denismaier commented 6 months ago

Ok. I think I understand. It looks like you basically combine two plugins into one. Is that correct?

aborel commented 6 months ago

I'm sure there are better ways to do various things in this code, it's my first contribution to a Zotero plugin.

As for the question in the issue title: yes, the plugin works with Zotero 6 and 7. src/zotero-ocr.js is exclusively for Zotero 7, while src/chrome/content/zoteroocr.js handles Zotero 6.

denismaier commented 6 months ago

Thanks. So this means you bascially need to maintain the code twice, right? Say you add an option, you'll add it to the legacy overlay code, and to the bootstrapped code. Or, if you want to change the processing logic, you'll have to do that in two locations. Sounds a bit cumbersome... Do you already have plans how you'll proceed once Zotero 7 is released?

stweil commented 6 months ago

The implementation for Zotero could also be upgraded to use the bootstrapped code. That would remove the duplicated code, but requires some more efforts to get code which works with both Zotero versions.

What would you propose for future releases after a stable Zotero 7 was released? Is support for Zotero 6 then still desired, or should it be dropped? Or do you have other ideas?

aborel commented 6 months ago

The Zotero 6 version has been around for a while, I'm a relatively new contributor and I worked on the Zotero 7 support. But I had zero interest in learning to support Zotero 6, my sincere hope is that Zotero 7 comes out of beta soon, and we can eventually remove the Zotero 6 code.

I don't think there are any functional evolutions planned for the near future, and bugfixes (versions 0.7.1 and 0.7.2) have been mostly for smaller issues in my own code, so there hasn't been any duplication of effort so far. There will probably be a discussion with the original developers at some point.

zuphilip commented 5 months ago

Yes I agree. It is probably not worth to spend more resource into the plugin development for Zotero 6. The current version works also for v6 and will still continue to do that even after Zotero 7 is released. Any new feature or plugin development can therefore focus on supporting v7 only.

denismaier commented 5 months ago

Thanks for the responses. I've now decided to use a similar approach in plugin. I also use a dual-version plugin architecture for now, and plan to remove the the legacy code at some point once Z7 has been released.