Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browser extension doesn't work with local app install on MacOS #9474

Closed
2 tasks done
allydunham opened this issue Dec 19, 2022 · 5 comments · Fixed by #9487
Closed
2 tasks done

Browser extension doesn't work with local app install on MacOS #9474

allydunham opened this issue Dec 19, 2022 · 5 comments · Fixed by #9487

Comments

@allydunham
Copy link
Contributor

JabRef version

5.8 (latest release)

Operating system

macOS

Details on version and operating system

OSX 12.6

Checked with the latest development build

  • I made a backup of my libraries before testing the latest development version.
  • I have tested the latest development version and the problem persists

Steps to reproduce the behaviour

Importing references through the browser extension fails when JabRef is installed to ~/Applications on MacOS. This initally seems like it occurs because org.jabref.jabref.json points to the wrong path but the issue continues after this is fixed because the hardcoded path in jabrefHost.py points to the system Applications directory.

Steps to reproduce:

  1. Install JabRef to ~/Applications
  2. Install browser extension (tested on firefox)
  3. Try to import a reference

I manually fix this each install by hardcoding the application path in jabrefHost.py to point to ~/Applications/JabRef.app/Contents/MacOS/JabRef. Similarly the correct path must be set in the JSON when installing the browser extension itself.

I noticed the comment above the path in jabrefHost.py (which doesn't work correctly on Mac) so possibly it's hard to automatically detect the location. Similarly, the JSON is necessarily hardcoded I imagine. If so, it would at least be useful to say for how to correct the problem in the browser extension install instructions.

Appendix

...

Only a generic error is produced:

Screenshot 2022-12-19 at 10 55 34

@Siedlerchr
Copy link
Member

Hi, thanks for the feedback. It would be great if you could add a section here:
https://docs.jabref.org/collect/jabref-browser-extension#mac-os

@allydunham
Copy link
Contributor Author

I have also noticed now that I don't need to hardcode the path, this change automatically finds the application path for me:

script_dir = Path(__file__).resolve().parent.parent
JABREF_PATH = script_dir / "bin/JabRef"
if not JABREF_PATH.exists():
    JABREF_PATH = script_dir / "MacOS/JabRef"

I'm not able to test it on other platforms, although I don't see why it wouldn't work as only the OSX part is changed.

I'm happy to add some notes to the docs as well, either for updating just the JSON or the application path in the script as well, depending on if you change the py script. Where would I do that?

@Siedlerchr
Copy link
Member

So it seems like the modifying the py script directly is the easiest option and would work for all other cases as well. It is located here https://github.com/JabRef/jabref/blob/main/buildres/mac/jabrefHost.py

Then it would only be necessary for the user to adjust the json file pointing to the right directory

@allydunham
Copy link
Contributor Author

That would be much easier than the current situation too - any edits to the script are overwriten when JabRef updates, whereas the json doesn't seem to be update often, at least in my experiance.

I'm still happy to write up changes to the docs and/or script when I have time, if that's useful? Although as I said I don't really have the setup/expertise to properly test the script changes, so might be easier for someone else.

@Siedlerchr
Copy link
Member

I think we can modify the python script based on your suggestion earlier so that in the next version of JabRef users don't have to adjust it. So the user would only need to adjust the json file. The python script in that folder I linked is only for mac. So feel free to make a PR with the changes to the python file

And then just add a note to the docs for modifying the json file

allydunham added a commit to allydunham/jabref that referenced this issue Dec 20, 2022
Following issue [9474](JabRef#9474).

The previous MacOS hardcoded path fails if JabRef is installed to a users local applications directory rather than the system one (e.g. if the user doesn't have admin permissions). On my version of MacOS (12.6.1) this change works correctly for local installs as well. The Mac app bundle is structured differently so an alternate path is still required.

If the difference in `which` behaviour is still a problem (maybe on older versions?) the original hardcoded fallback could also be included as a final option. If that is still necessary the explanatory comment should be updated too.

As mentioned on the issue thread the python script path in the browser extension JSON would still need to be updated for local installs, so I will submit a separate PR to the docs with notes on the process. If this script can't be updated I can expand the docs update to detail the hardcoded path change required to make the browser extension work for local app installs.
Siedlerchr pushed a commit that referenced this issue Dec 21, 2022
Following issue [9474](#9474).

The previous MacOS hardcoded path fails if JabRef is installed to a users local applications directory rather than the system one (e.g. if the user doesn't have admin permissions). On my version of MacOS (12.6.1) this change works correctly for local installs as well. The Mac app bundle is structured differently so an alternate path is still required.

If the difference in `which` behaviour is still a problem (maybe on older versions?) the original hardcoded fallback could also be included as a final option. If that is still necessary the explanatory comment should be updated too.

As mentioned on the issue thread the python script path in the browser extension JSON would still need to be updated for local installs, so I will submit a separate PR to the docs with notes on the process. If this script can't be updated I can expand the docs update to detail the hardcoded path change required to make the browser extension work for local app installs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants