Skip to content

Commit

Permalink
Display chrome:// urls as brave:// in the LocationBar
Browse files Browse the repository at this point in the history
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
  • Loading branch information
petemill authored and bbondy committed Oct 12, 2018
1 parent e889b42 commit d54259f
Show file tree
Hide file tree
Showing 9 changed files with 165 additions and 4 deletions.
3 changes: 3 additions & 0 deletions chromium_src/chrome/browser/ui/browser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
#include "chrome/browser/ui/browser_content_setting_bubble_model_delegate.h"
#include "brave/browser/ui/brave_browser_content_setting_bubble_model_delegate.h"
#include "brave/browser/ui/brave_browser_command_controller.h"
#include "brave/components/toolbar/brave_toolbar_model_impl.h"

#define ToolbarModelImpl BraveToolbarModelImpl
#define BrowserContentSettingBubbleModelDelegate BraveBrowserContentSettingBubbleModelDelegate
#define BrowserCommandController BraveBrowserCommandController
#include "../../../../../chrome/browser/ui/browser.cc"
#undef BrowserContentSettingBubbleModelDelegate
#undef BrowserCommandController
#undef ToolbarModelImpl
30 changes: 30 additions & 0 deletions components/toolbar/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
source_set("toolbar") {

sources = [
"brave_toolbar_model_impl.cc",
"brave_toolbar_model_impl.h",
"constants.cc",
"constants.h",
]

deps = [
"//base",
"//components/toolbar"
]

}

source_set("unit_tests") {
testonly = true
sources = [
"toolbar_model_impl_unittest.cc",
]

deps = [
":toolbar",
"//base",
"//components/toolbar",
"//testing/gtest",
"//url",
]
}
36 changes: 36 additions & 0 deletions components/toolbar/brave_toolbar_model_impl.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

#include "brave/components/toolbar/brave_toolbar_model_impl.h"

#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "brave/components/toolbar/constants.h"
#include "components/toolbar/toolbar_model_impl.h"

using namespace brave_toolbar;

namespace {

const base::string16 original_scheme_part =
base::ASCIIToUTF16(kOriginalInternalUIScheme);
const base::string16 replacement_scheme_part =
base::ASCIIToUTF16(kInternalUIScheme);

}

base::string16 BraveToolbarModelImpl::GetURLForDisplay() const {
base::string16 formatted_text = ToolbarModelImpl::GetURLForDisplay();

const GURL url(GetURL());
// Only replace chrome:// with brave:// if scheme is "chrome" and
// it has not been stripped from the display text
if (url.SchemeIs(kOriginalInternalUIScheme) &&
base::StartsWith(formatted_text, original_scheme_part,
base::CompareCase::INSENSITIVE_ASCII)) {
formatted_text.replace(0, original_scheme_part.length(),
replacement_scheme_part);
}
return formatted_text;
}
18 changes: 18 additions & 0 deletions components/toolbar/brave_toolbar_model_impl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

#ifndef BRAVE_COMPONENTS_TOOLBAR_BRAVE_TOOLBAR_MODEL_IMPL_H_
#define BRAVE_COMPONENTS_TOOLBAR_BRAVE_TOOLBAR_MODEL_IMPL_H_

#include "components/toolbar/toolbar_model_impl.h"

class BraveToolbarModelImpl : public ToolbarModelImpl {
public:
using ToolbarModelImpl::ToolbarModelImpl;
base::string16 GetURLForDisplay() const override;
private:
DISALLOW_COPY_AND_ASSIGN(BraveToolbarModelImpl);
};

#endif
10 changes: 10 additions & 0 deletions components/toolbar/constants.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

#include "brave/components/toolbar/constants.h"

namespace brave_toolbar {
const char kOriginalInternalUIScheme[] = "chrome";
const char kInternalUIScheme[] = "brave";
}
13 changes: 13 additions & 0 deletions components/toolbar/constants.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

#ifndef BRAVE_COMPONENTS_TOOLBAR_CONSTANTS_H_
#define BRAVE_COMPONENTS_TOOLBAR_CONSTANTS_H_

namespace brave_toolbar {
extern const char kOriginalInternalUIScheme[];
extern const char kInternalUIScheme[];
}

#endif
47 changes: 47 additions & 0 deletions components/toolbar/toolbar_model_impl_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// you can obtain one at http://mozilla.org/MPL/2.0/.

#include "brave/components/toolbar/brave_toolbar_model_impl.h"

#include "base/strings/utf_string_conversions.h"
#include "components/toolbar/toolbar_model_delegate.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"

namespace {

class FakeToolbarModelDelegate : public ToolbarModelDelegate {
public:
void SetURL(const GURL& url) { url_ = url; }

// ToolbarModelDelegate:
base::string16 FormattedStringWithEquivalentMeaning(
const GURL& url,
const base::string16& formatted_url) const override {
return formatted_url;
}

bool GetURL(GURL* url) const override {
*url = url_;
return true;
}

private:
GURL url_;
};

TEST(BraveToolbarModelImplTest,
DisplayUrlRewritesScheme) {
FakeToolbarModelDelegate delegate;
auto model = std::make_unique<BraveToolbarModelImpl>(&delegate, 1024);

delegate.SetURL(GURL("chrome://page"));

// Verify that both the full formatted URL and the display URL add the test
// suffix.
EXPECT_EQ(base::ASCIIToUTF16("brave://page"),
model->GetURLForDisplay());
}

} // namespace
11 changes: 7 additions & 4 deletions patches/chrome-browser-ui-BUILD.gn.patch
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index ea8b1a687eb567635a04e613b89b9a143b7c6af2..bc9ee6c3e5ac900df61a989661066039d4588da1 100644
index ea8b1a687eb567635a04e613b89b9a143b7c6af2..5e43eddc4274bfbe8d618ba5eaa0bb4648d1b9d4 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -1010,6 +1010,7 @@ jumbo_split_static_library("ui") {
@@ -1010,6 +1010,10 @@ jumbo_split_static_library("ui") {
"//ui/webui",
"//v8:v8_version",
]
+ deps += [ "//brave/browser/ui" ]
+ deps += [
+ "//brave/browser/ui",
+ "//brave/components/toolbar",
+ ]
allow_circular_includes_from +=
[ "//chrome/browser/ui/webui/bluetooth_internals" ]

@@ -2649,10 +2650,13 @@ jumbo_split_static_library("ui") {
@@ -2649,10 +2653,13 @@ jumbo_split_static_library("ui") {
]
deps += [ "//google_update" ]
} else {
Expand Down
1 change: 1 addition & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ test("brave_unit_tests") {
"//chrome/test:test_support",
"//components/prefs",
"//components/prefs:test_support",
"//brave/components/toolbar:unit_tests",
"//components/version_info",
"//content/test:test_support",
"//components/signin/core/browser",
Expand Down

0 comments on commit d54259f

Please sign in to comment.