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 settings link points to brave://settings rather than speedreader settings #17216

Closed
btlechowski opened this issue Jul 28, 2021 · 2 comments · Fixed by brave/brave-core#9580
Closed

speedreader settings link points to brave://settings rather than speedreader settings #17216

btlechowski opened this issue Jul 28, 2021 · 2 comments · Fixed by brave/brave-core#9580
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/exclude
Milestone
1.33.x - Release

Comments

@btlechowski
Copy link

Follow up to #9262

Steps to Reproduce

  1. Open https://www.nytimes.com/2021/07/28/upshot/delta-virus-economy-analysis.html
  2. Click speedreader icon in the address bar
  3. Click Enable Speedreader
  4. Click speedreader icon in the address bar
  5. Click Settings

Actual result:

The link opens brave://settings
image

Expected result:

The link opens speadreader settings
image

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

Brave 1.28.92 Chromium: 92.0.4515.107 (Official Build) beta (64-bit)
Revision 87a818b10553a07434ea9e2b6dccf3cbe7895134-refs/branch-heads/4515@{#1634}
OS Ubuntu 18.04 LTS

cc @keur @karenkliu @rebron @brave/legacy_qa
@btlechowski btlechowski mentioned this issue Jul 28, 2021
Closed
@kkuehlz
Copy link
Contributor

kkuehlz commented Jul 28, 2021

Thanks. I'll take this

kkuehlz pushed a commit to brave/brave-core that referenced this issue Jul 28, 2021
speedreader: Point to speedreader in settings
b50936c 
We have two options here: (1) Point to
brave://settings?search=speedreader, which will highlight the
Speedreader setting, or (2) or provide a pointer to
brave://settings/appearance. I went with the former because it makes the
setting a bit easier to identify. There isn't a way to link to a
specific setting.

Resolves brave/brave-browser#17216
@kkuehlz kkuehlz mentioned this issue Jul 28, 2021
Merged
24 tasks
kkuehlz pushed a commit to brave/brave-core that referenced this issue Jul 28, 2021
speedreader: Point to speedreader in settings
c31f378 
We have two options here: (1) Point to
brave://settings?search=speedreader, which will highlight the
Speedreader setting, or (2) or provide a pointer to
brave://settings/appearance. I went with the former because it makes the
setting a bit easier to identify. There isn't a way to link to a
specific setting.

Resolves brave/brave-browser#17216
@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Jul 28, 2021
rillian pushed a commit to brave/brave-core that referenced this issue Oct 29, 2021
@rillian
speedreader: Point to speedreader in settings
a61a9ca 
We have two options here: (1) Point to
brave://settings?search=speedreader, which will highlight the
Speedreader setting, or (2) or provide a pointer to
brave://settings/appearance. I went with the former because it makes the
setting a bit easier to identify. There isn't a way to link to a
specific setting.

Resolves brave/brave-browser#17216
@rillian rillian added this to the 1.33.x - Nightly milestone Nov 3, 2021
@stephendonner
Copy link

stephendonner commented Nov 3, 2021 •
edited
Loading

Verified PASSED using

Brave 1.33.58 Chromium: 95.0.4638.69 (Official Build) nightly (x86_64)
Revision 6a1600ed572fedecd573b6c2b90a22fe6392a410-refs/branch-heads/4638@{#984}
OS macOS Version 11.6.1 (Build 20G224)

NOTE: While testing this, I found and filed #19218, which seems to apply to all brave://settings/?search=foo URLs (requiring a window resize or scroll, to see the highlighted pref).

Behavior is at least better than before, so I'm passing this since the above followup bug will (hopefully) either address it for all the above cases, or we'll open a new bug for a different approach, but this correctly highlights the string.

Case 1: en-US (English)

Steps:

  1. new profile
  2. launched Brave
  3. opened https://www.nytimes.com/2021/07/28/upshot/delta-virus-economy-analysis.html
  4. clicked speedreader icon in the address bar
  5. clicked Enable Speedreader
  6. clicked speedreader icon in the address bar
  7. clicked Settings
default after slight scroll
Screen Shot 2021-11-03 at 1 32 20 PM Screen Shot 2021-11-03 at 1 33 18 PM

Case 2: CA (Catalan/Català)

Steps:

  1. set Catalan (Català) as my OS language
  2. also set it as my primary language in Brave's brave://settings/languages
  3. new profile
  4. launched Brave
  5. opened https://www.nytimes.com/2021/07/28/upshot/delta-virus-economy-analysis.html
  6. clicked speedreader icon in the address bar
  7. clicked Enable Speedreader
  8. clicked speedreader icon in the address bar
  9. clicked Settings
default after slight scroll
Captura de Pantalla 2021-11-03 a les 1 28 16 p  m Captura de Pantalla 2021-11-03 a les 4 25 23 p  m

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/exclude
Projects
None yet
Milestone
1.33.x - Release
Development

Successfully merging a pull request may close this issue.

speedreader: Point to speedreader in settings brave/brave-core
5 participants
@rillian @stephendonner @rebron @kkuehlz @btlechowski