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

New Tab Page uses 'chrome' links for History, Bookmarks, and Preferences #2566

Closed
bradleyrichter opened this issue Dec 14, 2018 · 13 comments · Fixed by brave/brave-core#1825 or brave/brave-core#15336
Closed

New Tab Page uses 'chrome' links for History, Bookmarks, and Preferences #2566

bradleyrichter opened this issue Dec 14, 2018 · 13 comments · Fixed by brave/brave-core#1825 or brave/brave-core#15336
Assignees
@petemill
Labels
branding bug feature/new-tab OS/Desktop priority/P4 Planned work. We expect to get to it "soon". QA Pass-macOS QA/Test-Plan-Specified QA/Yes release-notes/include
Milestone
1.47.x - Release

Comments

@bradleyrichter
Copy link

image
@bradleyrichter bradleyrichter added bug needs-text-change This change requires some careful wording. labels Dec 14, 2018
@srirambv
Copy link
Contributor

probably #1616 will fix this as well

@NejcZdovc NejcZdovc added this to the 1.x Backlog milestone Jan 2, 2019
@fmarier
Copy link
Member

fmarier commented Jan 16, 2019

Sadly, my fix for #1616 (brave/brave-core#1354) won't help with this issue.

@simonhong
Copy link
Member

simonhong commented Jan 18, 2019 •
edited
Loading

I think we can fix this by overriding StatusBubbleViews::SetURL() by mapping url from chrome to brave. Or can be resolved by fixing current brave to chrome scheme mapping timing.

@rebron rebron added the priority/P4 Planned work. We expect to get to it "soon". label Jan 24, 2019
@rebron rebron removed this from the 1.x Backlog milestone Feb 7, 2019
@rebron rebron assigned simonhong and unassigned rebron Mar 1, 2019
@simonhong simonhong added this to the 0.63.x - Nightly milestone Mar 3, 2019
@simonhong simonhong mentioned this issue Mar 3, 2019
Merged
19 tasks
@simonhong simonhong mentioned this issue Mar 13, 2019
Closed
@bsclifton bsclifton mentioned this issue Apr 9, 2019
Merged
19 tasks
bsclifton added a commit to brave/brave-core that referenced this issue Apr 9, 2019
@bsclifton
Revert #1825 because of unit test failures
a78275d 
Unfixes brave/brave-browser#2566

This reverts commit eba28d0, reversing
changes made to 94401f9.
@bsclifton
Copy link
Member

Reverted with brave/brave-core#2188 because there have been unit test issues. When brought up on Slack, @bridiver noticed some issues too. @simonhong can you re-open the original PR and/or hit up @bridiver? Thanks 😄

Removing milestone as code has been reverted

@bsclifton bsclifton reopened this Apr 9, 2019
@bsclifton bsclifton removed this from the 0.64.x - Dev milestone Apr 9, 2019
pilgrim-brave pushed a commit to brave/brave-core that referenced this issue Apr 16, 2019
@bsclifton @pilgrim-brave
Revert #1825 because of unit test failures
c9aed4e 
Unfixes brave/brave-browser#2566

This reverts commit eba28d0, reversing
changes made to 94401f9.
@rebron rebron mentioned this issue Feb 7, 2020
Closed
@rebron rebron mentioned this issue Jan 15, 2021
Closed
@rebron
Copy link
Collaborator

rebron commented Jan 15, 2021

+1 for @pes10k for this one via #13467

@rebron rebron removed the needs-text-change This change requires some careful wording. label Jan 15, 2021
@simonhong simonhong mentioned this issue Jan 23, 2021
Closed
@bsclifton bsclifton mentioned this issue Mar 8, 2021
Closed
@bsclifton bsclifton changed the title hover link on New Tab settings button shows chrome:// instead of brave:// New Tab Page uses 'chrome' links for History, Bookmarks, and Preferences Jul 26, 2021
@bsclifton
Copy link
Member

+1 from @JimB-Dev via #17156

@bsclifton bsclifton mentioned this issue Jul 26, 2021
Closed
@rebron
Copy link
Collaborator

rebron commented Sep 23, 2022

+1 from @uraashif #25467

@rebron rebron mentioned this issue Sep 23, 2022
Closed
@bridiver
Copy link
Contributor

Personally I think forcing everything to brave:// is a mistake. There are so many hardcoded usages of chrome:// in the code and sometimes it's a pain to get everything right and potentially introduces security issues.

@pes10k
Copy link
Contributor

pes10k commented Sep 24, 2022

@bridiver , can we copy what Edge does here then? I believe edge completely replaces chrome:// -> edge:// IIRC

@bridiver
Copy link
Contributor

@bridiver , can we copy what Edge does here then? I believe edge completely replaces chrome:// -> edge:// IIRC

I have no idea what they do to accomplish that. They might do a search/replace on the entire codebase which isn't practical for us atm

@pes10k
Copy link
Contributor

pes10k commented Sep 24, 2022

not being totally serious but, couldn't we just #define chrome: brave: 😅

@bridiver
Copy link
Contributor

not being totally serious but, couldn't we just #define chrome: brave: 😅

Haha, well unfortunately that would not fix any of them because the pre-processor doesn't modify strings and you can't have a colon in a define

@petemill petemill self-assigned this Oct 3, 2022
@petemill petemill mentioned this issue Oct 4, 2022
Merged
25 tasks
@brave-builds brave-builds added this to the 1.47.x - Nightly milestone Nov 3, 2022
@stephendonner
Copy link

stephendonner commented Dec 2, 2022 •
edited
Loading

Verified PASSED using

Brave 1.47.108 Chromium: 108.0.5359.71 (Official Build) beta (x86_64)
Revision 1e0e3868ee06e91ad636a874420e3ca3ae3756ac-refs/branch-heads/5359@{#1016}
OS macOS Version 13.1 (Build 22C5059b)

Steps:

  1. installed 1.47.108
  2. launched Brave
  3. loaded each of the following pages
  4. hovered over an internal link

Confirmed former chrome:// links now point to brave:// internally

brave://about release
Screenshot 2022-12-02 at 11 04 01 AM Screenshot 2022-12-02 at 11 07 18 AM
new-tab page release
Screenshot 2022-12-02 at 10 58 17 AM Screenshot 2022-12-02 at 11 08 40 AM
brave://settings/ipfs release
Screenshot 2022-12-02 at 11 09 54 AM Screenshot 2022-12-02 at 11 11 20 AM
brave://history release
Screenshot 2022-12-02 at 11 17 25 AM Screenshot 2022-12-02 at 11 17 15 AM

@LaurenWags LaurenWags mentioned this issue Jan 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@petemill petemill

Labels
branding bug feature/new-tab OS/Desktop priority/P4 Planned work. We expect to get to it "soon". QA Pass-macOS QA/Test-Plan-Specified QA/Yes release-notes/include
Projects
None yet
Milestone
1.47.x - Release