-
Notifications
You must be signed in to change notification settings - Fork 47
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
Adds an option to locally fetch site website content #300
Conversation
Hi Adrien, |
f012d95
to
20d02a4
Compare
@Simounet i've just rebased and fixed most of the linter errors.
Do you know what's the best way to handle them ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
That's weird because I fixed it on the master branch with this commit: 32cb230 |
Sorry, I did not fetched the latest changes before rebasing. That's fixed. I've fixed the typos/conventions errors, improved the filtering of the pages and used an object to avoid duplication of the function. If all of that is ok for you I can then squash those changes. |
Do you send the tab's page DOM to wallabag server? (to clean it) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be done after that. Thanks again for your time.
Some websites like https://www.arretsurimages.net are Single Page Applications behind a Paywall. Those websites cannot be fetched by wallabag using the credentials API. This commits adds an option where you can list the websites that you want to fetch locally.
@nicosomb yes, the innerHTML of the body is retrived here : https://github.com/wallabag/wallabagger/pull/300/files#diff-bfbd252c728bc45cc06ffc1d7f8eb7b9d5489941ad1c9183d8708245cc257e1eR624 |
@Simounet i've just applied changes about the function/this and variable naming, then rebased/squashed the branch. |
I think it's enough. If I understand, you only do this new thing for selected websites? |
Yes, its only done for the sites listed in the configuration. It's usefull if the site is and SPA and/or if the site is behind a paywall that is not managed/can't be managed with wallabag's site credentials. |
I added a commit to bypass the cache for your local fetch and updated the translations. It's now on the |
But why this feature is not enabled for all the websites? |
It could be but at the moment, submitting an entry to wallabag through the right click context menu on a link action is not supported. You have to open it on a new tab before. |
Some websites like https://www.arretsurimages.net are Single Page Applications behind a Paywall.
Those websites cannot be fetched by wallabag using the credentials API.
This PR adds an option where you can list the websites that you want to fetch locally
The tab's page DOM will be used as content and the tab's title will be used as title.