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

feat: store sites that are rendered on the frontend #7060

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

pipoprods
Copy link

@pipoprods pipoprods commented Nov 9, 2023

Q A
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes/no
Documentation no
Translation no
CHANGELOG.md no
License MIT

closes #6737

This adds single-file-proxy integration.
The proxy will render sites in a headless Chromium, returning the resulting page content. This makes Wallabag able to store sites that are rendered in JS.

Two operating modes (in parameters.yml):

  1. Proxy only sites that are defined in a given list:
    single_file_proxy_url: https://single-file-proxy.your-domain.tld
    single_file_proxy_all: false
    single_file_proxy_sites_to_proxy:
        - https://www.blast-info.fr
        - https://www.arretssurimage.net
  1. Proxy all sites:
    single_file_proxy_url: https://single-file-proxy.your-domain.tld
    single_file_proxy_all: true
    single_file_proxy_sites_to_proxy:

The proxy itself will only process sites that are defined in his own whitelist. It will HTTP Redirect others.
It seems safe to proxy all (only drawback is the external dependency for overall operation of Wallabag).

Questions to team:

  1. I've added no test. Could you point me to where I should add them?
  2. I've added no doc. Same question as above ;)

Copy link
Member

@j0k3r j0k3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About tests: maybe you can add one more job into https://github.com/wallabag/wallabag/blob/master/.github/workflows/continuous-integration.yml where you choose one version of PHP, install/enable/run single-file-proxy

About the doc, it'll be at https://github.com/wallabag/doc but maybe you can wait for the PR to be ok before starting writing the doc.

src/Wallabag/CoreBundle/Helper/ContentProxy.php Outdated Show resolved Hide resolved
src/Wallabag/CoreBundle/Helper/ContentProxy.php Outdated Show resolved Hide resolved
app/config/wallabag.yml Outdated Show resolved Hide resolved
app/config/wallabag.yml Outdated Show resolved Hide resolved
Comment on lines 170 to 171
# - https://www.blast-info.fr
# - https://www.arretssurimage.net
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about app/config/parameters.yml.dist

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better as an application-level config, something like the Site credential management?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This point remains unfixed. I'll have a look at this app-level integration now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wallabag can't parse JS-rendered pages
3 participants