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

chrome:// urls should display as brave:// #1458

Closed
petemill opened this issue Oct 8, 2018 · 1 comment · Fixed by brave/brave-core#584
Closed

chrome:// urls should display as brave:// #1458

petemill opened this issue Oct 8, 2018 · 1 comment · Fixed by brave/brave-core#584

Comments

@petemill
Copy link
Member

petemill commented Oct 8, 2018

It may be too much work to redirect all chrome:// urls to brave:// and have the appropriate handlers and checks still work reliable.

Luckily, the ToolbarModel provides GetURLForDisplay which can be overridden so that brave:// displays most of the time (until the URL is edited).

When paired with #810 this should cover quite a bit of the requirement to support brave:// URLs without confusing the users about the chromium-based core.

Test plan:

Displays brave:// url

  • Clean Profile
  • Launch browser
  • Welcome page should be open with brave://welcome as the address in LocationBar

Edits chrome:// url

  • Open settings
  • Settings page should be open with brave:// prefix in LocationBar
  • Edit the url by focusing in the LocationBar and moving the cursor or adding text
  • LocationBar text should have chrome:// prefix

Copies chrome:// url

  • Open settings
  • Settings page should be open with brave:// prefix in LocationBar
  • Right-click the LocationBar text and select 'Copy'
  • Clipboard text should have chrome:// prefix
@petemill petemill added this to the 1.0 (0.56.x) milestone Oct 8, 2018
@petemill petemill self-assigned this Oct 8, 2018
bbondy pushed a commit to brave/brave-core that referenced this issue Oct 12, 2018
This only affects the DisplayURL in that, when the url is being edited or copied, the actual chrome://*** url will be present.
This seems acceptable given:
- These urls 99% of the time are for display purposes only, as a result of another UI action, and will not be typed manually
- We intend to support brave://*** actual urls, redirecting to chrome://*** actual URLs automatically, which will get displayed as brave://*** URLs (brave/brave-browser#810)

Fix brave/brave-browser#1458
petemill added a commit to brave/brave-core that referenced this issue Oct 13, 2018
This only affects the DisplayURL in that, when the url is being edited or copied, the actual chrome://*** url will be present.
This seems acceptable given:
- These urls 99% of the time are for display purposes only, as a result of another UI action, and will not be typed manually
- We intend to support brave://*** actual urls, redirecting to chrome://*** actual URLs automatically, which will get displayed as brave://*** URLs (brave/brave-browser#810)

Fix brave/brave-browser#1458
@bbondy bbondy modified the milestones: 0.56.x, 0.55.x Oct 14, 2018
@srirambv
Copy link
Contributor

srirambv commented Oct 16, 2018

Verification Passed on

Brave 0.55.14 Chromium: 70.0.3538.54 (Official Build) beta(64-bit)
Revision 4f8e578b6680574714e9ed3bb9f02922b4dde40d-refs/branch-heads/3538@{#937}
OS Linux

image

Went through verification using the following build under macOS 10.13.6 x64 - PASSED

Brave 0.55.14 Chromium: 70.0.3538.54 (Official Build) beta(64-bit)
Revision 4f8e578b6680574714e9ed3bb9f02922b4dde40d-refs/branch-heads/3538@{#937}
OS Mac OS X
  • went through about 70% of the links under brave://about and ensured they worked with brave://

Verification Passed on

Brave 0.55.14 Chromium: 70.0.3538.54 (Official Build) beta (64-bit)
Revision 4f8e578b6680574714e9ed3bb9f02922b4dde40d-refs/branch-heads/3538@{#937}
OS Windows

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