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

Allow user to specify speedreader backend #14959

Closed
kkuehlz opened this issue Mar 25, 2021 · 3 comments · Fixed by brave/brave-core#8363
Closed

Allow user to specify speedreader backend #14959

kkuehlz opened this issue Mar 25, 2021 · 3 comments · Fixed by brave/brave-core#8363
Assignees
Labels
feature/speedreader OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA/No release-notes/exclude

Comments

@kkuehlz
Copy link
Contributor

kkuehlz commented Mar 25, 2021

Add a command line switch --speedreader-backend=[streaming|heuristics] so that @pes10k and @rebron can give nightly feedback on the separate backends. If not specified, it will default to streaming, which preserves the existing behavior of speedreader.

@kkuehlz kkuehlz added priority/P3 The next thing for us to work on. It'll ride the trains. QA/No release-notes/exclude feature/speedreader labels Mar 25, 2021
@kkuehlz kkuehlz self-assigned this Mar 25, 2021
@pes10k
Copy link
Contributor

pes10k commented Mar 25, 2021

Terrific! @keur to just make sure i understand correctly, is this right:

  • streaming: use the list, and if the list is silent about the URL, wait until the page has loaded and then classify
  • heuristics: use the list, and if the list is silent about the URL, buffer the page, classify, and either present the speedreader version, or the standard page

@kkuehlz
Copy link
Contributor Author

kkuehlz commented Mar 25, 2021

@pes10k

  • streaming: use the list, and if the list is silent about the URL, wait until the page has loaded and then classify

Streaming uses this list. Each domain + path has an associated set of adblock rules that are used to rewrite the page. This doesn't use classification or the Arc90 algorithm. This is how Speedreader is currently is deployed on nightly and beta with no changes. This way the behavior of Speedreader doesn't change at all if the switch is not supplied.

  • heuristics: use the list, and if the list is silent about the URL, buffer the page, classify, and either present the speedreader version, or the standard page

No list. Classify everything, then use Arc90 if the page is classified as readable. brave/brave-core#8349 adds some (very important) missing pieces to our implementation of Arc90. Since the classification can cause a delayed paint, that's why we guard this one behind the flag (for now). I have some ideas for how to get around this, but we can focus on that once we are super happy with the results.

@pes10k
Copy link
Contributor

pes10k commented Mar 26, 2021

Moving to the PR / review

Gotcha, thanks for clarifying. Could you change the names to something more descriptive then?

Maybe list, classify (and if there is such an option, list-and-classify)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/speedreader OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA/No release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants