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.

Sign up for GitHub

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

Jump to bottom

speedreader should pick shortest description #18056

Closed
kkuehlz opened this issue Sep 13, 2021 · 2 comments · Fixed by brave/brave-core#12731
Closed

speedreader should pick shortest description #18056

kkuehlz opened this issue Sep 13, 2021 · 2 comments · Fixed by brave/brave-core#12731
Assignees
@boocmp
Labels
feature/speedreader OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/exclude
Milestone
1.39.x - Release

Comments

@kkuehlz
Copy link
Contributor

kkuehlz commented Sep 13, 2021

https://variety.com/2021/tv/news/abc-new-logo-2021-1235059302/ has multiple descriptions. It looks like we always preference the JSON-LD description when we should instead be finding the shortest of the descriptions provided by json-ld and meta tags.
@kkuehlz kkuehlz self-assigned this Sep 13, 2021
@kkuehlz kkuehlz added feature/speedreader OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. labels Sep 13, 2021
@kkuehlz kkuehlz mentioned this issue Sep 13, 2021
Closed
@boocmp boocmp assigned boocmp and unassigned kkuehlz Mar 23, 2022
@boocmp boocmp mentioned this issue Mar 24, 2022
Merged
25 tasks
@boocmp boocmp added this to the 1.39.x - Nightly milestone Mar 25, 2022
@stephendonner
Copy link

Verified PASSED using

Brave 1.39.10 Chromium: 100.0.4896.46 (Official Build) nightly (x86_64)
Revision 5ca33821b2211805855c77d334353d27c616a7ca-refs/branch-heads/4896@{#584}
OS macOS Version 11.6.5 (Build 20G527)

Confirmed that loading https://variety.com/2021/tv/news/abc-new-logo-2021-1235059302/ in beta shows a longer summary, vs. nightly which is much shorter, as per the description.

beta vs. nightly

Screen Shot 2022-03-28 at 3 53 47 PM

@MadhaviSeelam
Copy link

MadhaviSeelam commented Mar 29, 2022 •
edited
Loading

Brave 1.39.14 Chromium: 100.0.4896.60 (Official Build) nightly (64-bit)
Revision 6a5d10861ce8de5fce22564658033b43cb7de047-refs/branch-heads/4896@{#875}
OS Windows 11 Version 21H2 (Build 22000.556)

Verified the fix in nightly that loading https://variety.com/2021/tv/news/abc-new-logo-2021-1235059302/ shows much shorter summary and beta shows a longer summary, as per the description.

nigthly vs beta

speed reader

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@boocmp boocmp

Labels
feature/speedreader OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/exclude
Projects
None yet
Milestone
1.39.x - Release
Development

Successfully merging a pull request may close this issue.

Select shortest description instead of preferring json+ld. Added re… brave/brave-core
4 participants
@stephendonner @kkuehlz @boocmp @MadhaviSeelam