pgaskin / ePubViewer

ePub viewer with dictionary, themes, search, offline support, and more
http://pgaskin.net/ePubViewer
MIT License
350 stars 78 forks source link

Missing `)` in injected script #19

Closed aledemajo closed 4 years ago

aledemajo commented 4 years ago

Hi Patrick,

First of all, awesome job with ePubViewer!!! Thank you for your hard work and permissive license 🙏

I want to address two quick issues on the repo

quick bug fix

I cloned and ran the repo locally, everything works as expected :) I did notice one small syntax error in the console, which seems to be coming from an injected JS file pulled from your site

The following snippet is missing a )

(window.location.toString().indexOf(atob("XXXXXXXX") > -1) {
    //...
}

XXXXXXXX is an example, of course

I'm not exactly sure how relevant this piece is, as it seems to be checking for a license on an open source solution 🤔 but figured I would bring it up. Sorry I couldn't open a PR with the change, I obviously don't have access to this file

personal forks and changes

I also noticed a log in the injected script

If you are planning to make changes to your own copy of ePubViewer, it would be nice if you could remove the Sentry error reporting (all the Raven stuff). Thanks!

Which did not print to my console when running your repository locally (directly in the browser, not through a web server)

Similar to my previous point, not sure how relevant it is

More importantly: is there any other functionality that you would like developers to remove from their personal forks of ePubViewer?

If so, we could add them to a local development section of the readme. I can make a PR for it if you point me in the right direction!

Thanks again for your hard work and feel free to close the issue if you find these notes not relevant :)

pgaskin commented 4 years ago

I mainly hacked that together quickly because people started using ePubViewer as-is in other applications, making changes, cluttering the error capturing, claiming it as theirs, and so on. I needed a way to detect it and communicate with those users. I'll fix the script later today.

I don't have anything against people using it as-is or with modifications, but I'd prefer if they remove Sentry and GA.