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 titles are being cut short #17983

Closed
kkuehlz opened this issue Sep 9, 2021 · 6 comments · Fixed by brave/brave-core#10027
Closed

Speedreader titles are being cut short #17983

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

Comments

@kkuehlz
Copy link
Contributor

kkuehlz commented Sep 9, 2021 •
edited
Loading

We handle description truncation naively. Ensure that we don't mistake sentence terminators with common abbreviations.

Reported by @janypiotrowski for this site. Getting cut off on U.S. https://www.usnews.com/news/national-news/articles/2021-08-30/the-war-in-afghanistan-ends-where-it-started
@kkuehlz kkuehlz self-assigned this Sep 9, 2021
@kkuehlz kkuehlz added feature/speedreader OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes release-notes/include labels Sep 9, 2021
@kkuehlz
Copy link
Contributor Author

kkuehlz commented Sep 9, 2021

Test Plan

Ensure the description on the above site isn't truncated mid-sentence.

@kkuehlz kkuehlz mentioned this issue Sep 9, 2021
Merged
24 tasks
kkuehlz pushed a commit to brave/brave-core that referenced this issue Sep 10, 2021
speedreader: Improve description truncation
098bc3c 
For sentences that are more than 200 characters (roughly 3 sentences)
truncate the result at the first sentence. Right now some descriptions
are getting prematurely truncated on abbreviations.

Resolves brave/brave-browser#17983
@kkuehlz kkuehlz added this to the 1.31.x - Nightly milestone Sep 10, 2021
@stephendonner
Copy link

stephendonner commented Sep 11, 2021 •
edited
Loading

@brave/legacy_qa when verifying, we can also use https://www.sfgate.com/hawaii/article/Hawaii-Haiku-Stairs-removal-advances-Honolulu-16449522.php?IPID=SFGate-HP-CP-Spotlight and https://variety.com/2021/tv/news/abc-new-logo-2021-1235059302/ to test

example example
Screen Shot 2021-09-10 at 6 40 42 PM Screen Shot 2021-09-10 at 6 43 43 PM

@stephendonner
Copy link

@keur the above two sites: do they have the same issue or a different issue? They're not fixed, and I'm not sure if they should be here, or if I should file a new issue; thanks!

@kkuehlz
Copy link
Contributor Author

kkuehlz commented Sep 13, 2021 •
edited
Loading

@stephendonner the SFGate is behaving as expected. If you view source on it and search for meta name="description", they provide the description with the ellipsis. For the Variety article the same is true, but there are some shorter titles which we should preference. That's a separate issue though, since neither is being truncated by Speedreader.

@kkuehlz
Copy link
Contributor Author

kkuehlz commented Sep 13, 2021

Followup for the Variety article is here #18056

@stephendonner
Copy link

Verified PASSED using

Brave 1.31.37 Chromium: 93.0.4577.63 (Official Build) nightly (x86_64)
Revision ff5c0da2ec0adeaed5550e6c7e98417dac77d98a-refs/branch-heads/4577@{#1135}
OS macOS Version 11.5.2 (Build 20G95)

Steps:

  1. new profile
  2. launched Brave
  3. loaded https://www.usnews.com/news/national-news/articles/2021-08-30/the-war-in-afghanistan-ends-where-it-started
  4. enabled Speedreader mode

Confirmed the summary text beneath the title is all intact, now.

before fix after fix
image Screen Shot 2021-09-13 at 2 43 41 PM

@kjozwiak kjozwiak mentioned this issue Oct 19, 2021
Closed
@rebron rebron changed the title speedreader titles are being cut short Speedreader titles are being cut short Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kkuehlz kkuehlz

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

Successfully merging a pull request may close this issue.

speedreader: Improve description truncation brave/brave-core
2 participants
@stephendonner @kkuehlz