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

annotation json contains hardcoded URI referencing demo pdf instead of annotation target #7

Closed
caseyg opened this issue Aug 28, 2021 · 13 comments

Comments

@caseyg
Copy link

@caseyg caseyg commented Aug 28, 2021

Note that the arxiv link in the json has nothing to do with the annotation target I've selected

Screenshot 2021-08-28 at 9 03 37 AM

Maybe related to this?

`https://via.hypothes.is/proxy/static/xP1ZVAo-CVhW7kwNneW_oQ/1628964000/https://arxiv.org/pdf/1702.08734.pdf`

@elias-sundqvist
Copy link
Owner

@elias-sundqvist elias-sundqvist commented Aug 28, 2021

Yes, this is an artifact of how things are implemented. I'll have to think about ways to improve it...

In the current implementation, I have tried to modify the hypothes.is code as little as possible. My reasoning behind this decision is that upgrading to newer versions of hypothes.is as they are released should be simple.

The pdf loading currently works by replacing the response when hypothes.is tries to fetch the demo file.
So hypothes.is always "thinks" that the file it gets is https://arxiv.org/pdf/1702.08734.pdf. But it actually gets the file specified by the user.

The content of the annotation-json block is the exact data that I get from the hypothes.is REST API.
One option could be to modify the data before saving it to the markdown file. There is a risk that that would introduce new issues though.

@GitMurf
Copy link

@GitMurf GitMurf commented Aug 30, 2021

@elias-sundqvist when you say:

The content of the annotation-json block is the exact data that I get from the hypothes.is REST API.

Does this mean our local PDFs / highlights / annotations are being "sent" / processed through hyopthesis.is REST API? I had thought everything was done locally just using hypothesis.is github repo library to handle all the UI/UX and processing of PDF annotations etc.

In summary, just wondering if this plugin is staying "local" 100% of the time? Or if it is making calls / using hypothesis.is server side stuff to do things?

Thanks!

@elias-sundqvist
Copy link
Owner

@elias-sundqvist elias-sundqvist commented Aug 30, 2021

No, nothing is sent to hypothes.is. The plugin should work completely offline. I just use the REST api to communicate between my own plugin code and the local hypothesis.is instance.

@GitMurf
Copy link

@GitMurf GitMurf commented Aug 30, 2021

So I tested by turning my internet completely off and the highlighting and everything worked fine for me. But I did get these hypothesis.is connection errors. Is this just because of your original "fake call" you do with the arxiv address and then replace the result with the local PDF like you mentioned above? Or is there any other reason it is communication with hypothesis.is each time?

This isn't a complaint or even an "issue" necessarily, but just want to be clear because a lot of folks saw this as a way to do PDF annotations without any connection to the internet and no communication to any non local servers. Thanks!

image

@elias-sundqvist
Copy link
Owner

@elias-sundqvist elias-sundqvist commented Aug 30, 2021

So, technically, some calls to hypothes.is are attempted, but they don't do anything.

Before the iframe patch is applied, it tries to connect to the real hypothes.is website. That request is cancelled as soon as the patch is applied though.

The errors you screenshot above are from the websocket API.
I haven't patched that, so it is just ignored. It's not very important though. I believe it can be used to update the annotation list in real-time. Otherwise the REST api covers all functionality.

@GitMurf
Copy link

@GitMurf GitMurf commented Aug 30, 2021

@elias-sundqvist makes sense. Just trying to get all the questions out before the masses start to use and ask "is my data leaving my machine?" haha ;-) I just checked the network console and it looks like the websocket call is with that "dummy" arxiv.org PDF that you use. So that is probably what this is? The initial "fake call" with that dummy PDF and then you replace what is supposed to be returned from hypothesis with the local PDF, right?

image

image

@elias-sundqvist
Copy link
Owner

@elias-sundqvist elias-sundqvist commented Aug 30, 2021

I'm not too sure about what is being communicated through the websocket api. I should probably replace it with a dummy object to get rid of the error messages and to make the "is the data leaving my machine?" people happy.

@GitMurf
Copy link

@GitMurf GitMurf commented Aug 30, 2021

I'm not too sure about what is being communicated through the websocket api. I should probably replace it with a dummy object to get rid of the error messages and to make the "is the data leaving my machine?" people happy.

Yeah I hate to say it but you may as well get ahead of it ;-) Because part of the POWER of this plugin is being able to do with your local PDFs and annotate to a local MD and not have to rely on or trust any web hosting / web app with your data / security / privacy. Folks like me will want to use this to highlight and markup SUPER sensitive contracts with clients etc. and will want 0% question on whether something could be leaving my machine haha... I know, pain in the ass! ;-)

For me, based off what you said above and my testing I am content with the fact nothing is leaving my machine. But I think it would just make it a whole lot easier answering these questions that will come up down the road if you are able to say 'Yes, turn your internet off and you will see there are no errors. And watch the network tab in the console as long as you want, you won't see any external calls" haha ;-)

Best of luck and appreciate all your amazing work!

Do you have a Buy me a Coffee link or any way to sponsor you / contribute?

@elias-sundqvist
Copy link
Owner

@elias-sundqvist elias-sundqvist commented Aug 30, 2021

True. I'll try to fix these things soon and keep privacy in mind.

Do you have a Buy me a Coffee link or any way to sponsor you / contribute?

You can sponsor me on Github or donate something on Paypal.

Thanks :)

@GitMurf
Copy link

@GitMurf GitMurf commented Aug 30, 2021

You can sponsor me on Github or donate something on Paypal.

Done! Keep up the great work :)

elias-sundqvist pushed a commit that referenced this issue Aug 30, 2021
@elias-sundqvist
Copy link
Owner

@elias-sundqvist elias-sundqvist commented Aug 30, 2021

Thanks for the donation :)
The new release has patched the websocket API (it's not used for anything yet though).
I also removed the newrelic performance tracker, and found a solution for getting around the initial iframe request.
There is still a request made to sentry.io for error tracking. But besides that, everything should be completely offline now.
I will try to find a reasonable method of removing sentry.io as well and include that in a later release.

@elias-sundqvist
Copy link
Owner

@elias-sundqvist elias-sundqvist commented Aug 30, 2021

The new release uses more reasonable URLs in the json blobs, so I will close this issue.

@GitMurf
Copy link

@GitMurf GitMurf commented Aug 30, 2021

@elias-sundqvist this is awesome!! Thanks so much. I saw the sentry thing too but don’t think that is a big deal at all so I wouldn’t put much time into trying to figure that one out. We have a group of people who chat daily about obsidian stuff and are super excited about this plugin and I’m hoping you will get some more sponsorship / donations soon :) very nice work!

elias-sundqvist pushed a commit that referenced this issue Sep 1, 2021
To reduce confusion about privacy.
Related to comments by @GitMurf in #7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants