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

Update Geolocation permission bubble #23278

Merged
merged 19 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,18 @@ Or change later at <ph name="SETTINGS_EXTENIONS_LINK">$2<ex>brave://settings/ext
<message name="IDS_CRASH_REPORT_PERMISSION_ASK_DIALOG_FOOTNOTE_TEXT_SETTING_PART" desc="This is used for replacing SETTING_TEXT from above FOOTNOTE_TEXT.">
this setting
</message>

<!-- Geolocation permission bubble -->
<message name="IDS_GEOLOCATION_PERMISSION_BUBBLE_LOW_ACCURACY_LABEL" desc="The text for geolocation permission bubble with low accuracy. Each part will be used to get offset in the string">
This site has requested your <ph name="PART_ONE">$1</ph>general location<ph name="PART_ONE_END">$1</ph>. <ph name="PART_TWO">$1</ph>Learn more<ph name="PART_TWO_END">$1</ph>
</message>
<message name="IDS_GEOLOCATION_PERMISSION_BUBBLE_HIGH_ACCURACY_WITH_LOCATION_SERVICE_LABEL" desc="The text for geolocation permission bubble with high accuracy. Each part will be used to get offset in the string">
This site has requested your <ph name="PART_ONE">$1</ph>precise location<ph name="PART_ONE_END">$1</ph>. <ph name="PART_TWO">$1</ph>Learn more<ph name="PART_TWO_END">$1</ph>
</message>
<message name="IDS_GEOLOCATION_PERMISSION_BUBBLE_HIGH_ACCURACY_WITHOUT_LOCATION_SERVICE_LABEL" desc="The text for geolocation permission bubble with high accuracy. Each part will be used to get offset in the string">
This site has requested your <ph name="PART_ONE">$1</ph>precise location<ph name="PART_ONE_END">$1</ph>. Brave will only able to provide <ph name="PART_TWO">$1</ph>general location<ph name="PART_TWO_END">$1</ph> data due to <ph name="PART_THREE">$1</ph>Location Services<ph name="PART_THREE_END">$1</ph> being disabled. <ph name="PART_FOUR">$1</ph>Learn more<ph name="PART_FOUR_END">$1</ph>
</message>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these strings should use the usual placeholders which then can be used to get the offsets to style the message. It will be clearer for translation and easier for everyone to understand.

cc @mkarolin

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added Each part will be used to get offset in the string. to the desc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of using placeholders like so, but maybe that's not the best way, I'm not sure.

IDS_GEOLOCATION_PERMISSION_BUBBLE_REQUESTED_LOCATION_TYPE_LABEL:
This site has requested your <ph name="LOCATION_TYPE">$1<ex>general location</ex></ph>. <ph name="LEARN_MORE">$2<ex>Learn more</ex></ph>.

IDS_GEOLOCATION_PERMISSION_BUBBLE_LOCATION_TYPE_GENERAL desc="Used as a placeholder replacement in IDS_GEOLOCATION_PERMISSION_BUBBLE_REQUESTED_LOCATION_TYPE_LABEL":
general location

IDS_GEOLOCATION_PERMISSION_BUBBLE_LOCATION_TYPE_PRECISE desc="Used as a placeholder replacement in IDS_GEOLOCATION_PERMISSION_BUBBLE_REQUESTED_LOCATION_TYPE_LABEL":
precise location

IDS_GEOLOCATION_PERMISSION_BUBBLE_REQUESTED_LOCATION_TYPE_WITHOUT_LOCATION_SERVICE_LABEL:
Brave will only able to provide <ph name="LOCATION_TYPE">$1<ex>general location</ex></ph> data due to <ph name="LOCATION_SERVICES">$2<ex>Location Services</ex></ph> being disabled. <ph name="LEARN_MORE">$2<ex>Learn more</ex></ph>

IDS_GEOLOCATION_PERMISSION_BUBBLE_LOCATION_SERVICES:
Location Services

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was what I tried in the first and @mkarolin suggested another way for better localization - #19529 (comment)


<!-- Closing confirm dialog -->
<message name="IDS_WINDOW_CLOSING_CONFIRM_DLG_OK_BUTTON_LABEL" desc="The text for ok button in the dialog">
Close all
Expand Down
13 changes: 13 additions & 0 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ using extensions::ChromeContentBrowserClientExtensionsPart;

#if !BUILDFLAG(IS_ANDROID)
#include "brave/browser/new_tab/new_tab_shows_navigation_throttle.h"
#include "brave/browser/ui/geolocation/brave_geolocation_permission_tab_helper.h"
#include "brave/browser/ui/webui/brave_news_internals/brave_news_internals_ui.h"
#include "brave/browser/ui/webui/brave_rewards/rewards_panel_ui.h"
#include "brave/browser/ui/webui/brave_rewards/tip_panel_ui.h"
Expand Down Expand Up @@ -564,6 +565,18 @@ void BraveContentBrowserClient::
&render_frame_host));
#endif // BUILDFLAG(ENABLE_WIDEVINE)

#if !BUILDFLAG(IS_ANDROID)
associated_registry.AddInterface<
geolocation::mojom::BraveGeolocationPermission>(base::BindRepeating(
[](content::RenderFrameHost* render_frame_host,
mojo::PendingAssociatedReceiver<
geolocation::mojom::BraveGeolocationPermission> receiver) {
BraveGeolocationPermissionTabHelper::BindBraveGeolocationPermission(
std::move(receiver), render_frame_host);
},
&render_frame_host));
#endif // !BUILDFLAG(IS_ANDROID)

associated_registry.AddInterface<
brave_shields::mojom::BraveShieldsHost>(base::BindRepeating(
[](content::RenderFrameHost* render_frame_host,
Expand Down
2 changes: 2 additions & 0 deletions browser/brave_tab_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@

#if !BUILDFLAG(IS_ANDROID)
#include "brave/browser/ui/brave_shields_data_controller.h"
#include "brave/browser/ui/geolocation/brave_geolocation_permission_tab_helper.h"
#include "chrome/browser/ui/thumbnails/thumbnail_tab_helper.h"
#endif

Expand Down Expand Up @@ -131,6 +132,7 @@ void AttachTabHelpers(content::WebContents* web_contents) {
BraveBookmarkTabHelper::CreateForWebContents(web_contents);
brave_shields::BraveShieldsDataController::CreateForWebContents(web_contents);
ThumbnailTabHelper::CreateForWebContents(web_contents);
BraveGeolocationPermissionTabHelper::CreateForWebContents(web_contents);
#endif

brave_rewards::RewardsTabHelper::CreateForWebContents(web_contents);
Expand Down
23 changes: 23 additions & 0 deletions browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,29 @@ source_set("ui") {
]
}

if (is_win || is_mac || is_linux) {
sources += [
"geolocation/brave_geolocation_permission_tab_helper.cc",
"geolocation/brave_geolocation_permission_tab_helper.h",
"geolocation/geolocation_utils.h",
]

deps += [ "//brave/components/brave_geolocation_permission/common:brave_geolocation_permission" ]

if (is_win) {
sources += [ "geolocation/geolocation_utils_win.cc" ]
}

if (is_mac) {
sources += [ "geolocation/geolocation_utils_mac.mm" ]
frameworks = [ "CoreLocation.framework" ]
}

if (is_linux) {
sources += [ "geolocation/geolocation_utils_linux.cc" ]
}
}

# brave's obsolete infobar is only used on Windows and Mac now.
if (is_win || is_mac) {
sources += [
Expand Down
24 changes: 24 additions & 0 deletions browser/ui/geolocation/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Copyright (c) 2024 The Brave Authors. All rights reserved.
# 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 https://mozilla.org/MPL/2.0/.

assert(!is_android)

source_set("browser_tests") {
testonly = true
defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]

sources = [ "brave_geolocation_browsertest.cc" ]
deps = [
"//base",
"//brave/components/constants",
"//chrome/browser",
"//chrome/browser/ui",
"//chrome/common:constants",
"//chrome/test:test_support_ui",
"//content/test:test_support",
"//net:test_support",
"//url",
]
}
80 changes: 80 additions & 0 deletions browser/ui/geolocation/brave_geolocation_browsertest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/* Copyright (c) 2024 The Brave Authors. All rights reserved.
* 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 https://mozilla.org/MPL/2.0/. */

#include <string>

#include "base/path_service.h"
#include "brave/browser/ui/geolocation/brave_geolocation_permission_tab_helper.h"
#include "brave/components/constants/brave_paths.h"
#include "chrome/browser/ssl/cert_verifier_browser_test.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h"
#include "content/public/test/test_utils.h"
#include "net/dns/mock_host_resolver.h"
#include "url/gurl.h"

class GeolocationPermissionRequestBrowserTest : public CertVerifierBrowserTest {
public:
GeolocationPermissionRequestBrowserTest()
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}

void SetUpOnMainThread() override {
CertVerifierBrowserTest::SetUpOnMainThread();
host_resolver()->AddRule("*", "127.0.0.1");
brave::RegisterPathProvider();
base::FilePath test_data_dir =
base::PathService::CheckedGet(brave::DIR_TEST_DATA);
https_server_.ServeFilesFromDirectory(test_data_dir);
mock_cert_verifier()->set_default_result(net::OK);

ASSERT_TRUE(https_server_.Start());
}

content::WebContents* active_contents() {
return browser()->tab_strip_model()->GetActiveWebContents();
}

protected:
net::EmbeddedTestServer https_server_;
};

IN_PROC_BROWSER_TEST_F(GeolocationPermissionRequestBrowserTest,
SetEnableHighAccuracyTest) {
GURL url = https_server_.GetURL("a.com", "/simple.html");
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));

auto* tab_helper =
BraveGeolocationPermissionTabHelper::FromWebContents(active_contents());
EXPECT_FALSE(tab_helper->enable_high_accuracy());

const std::string get_current_position_js_with_high =
"navigator.geolocation.getCurrentPosition(() => {}, () => {}, { "
"enableHighAccuracy : true })";
ASSERT_EQ(nullptr, content::EvalJs(active_contents(),
get_current_position_js_with_high));
content::RunAllTasksUntilIdle();
EXPECT_TRUE(tab_helper->enable_high_accuracy());

// Reload clear high_accuracy bit from tab helper.
content::TestNavigationObserver observer(active_contents());
chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB);
observer.Wait();
EXPECT_FALSE(tab_helper->enable_high_accuracy());

const std::string get_current_position_js_without_high =
"navigator.geolocation.getCurrentPosition(() => {}, () => {}, { "
"enableHighAccuracy : false })";
ASSERT_EQ(nullptr, content::EvalJs(active_contents(),
get_current_position_js_without_high));
content::RunAllTasksUntilIdle();
EXPECT_FALSE(tab_helper->enable_high_accuracy());
}
52 changes: 52 additions & 0 deletions browser/ui/geolocation/brave_geolocation_permission_tab_helper.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/* Copyright (c) 2024 The Brave Authors. All rights reserved.
* 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 https://mozilla.org/MPL/2.0/. */

#include "brave/browser/ui/geolocation/brave_geolocation_permission_tab_helper.h"

#include <utility>

#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h"

BraveGeolocationPermissionTabHelper::BraveGeolocationPermissionTabHelper(
content::WebContents* contents)
: WebContentsObserver(contents),
content::WebContentsUserData<BraveGeolocationPermissionTabHelper>(
*contents),
brave_geolocation_permission_receivers_(contents, this) {}

BraveGeolocationPermissionTabHelper::~BraveGeolocationPermissionTabHelper() =
default;

// static
void BraveGeolocationPermissionTabHelper::BindBraveGeolocationPermission(
mojo::PendingAssociatedReceiver<
geolocation::mojom::BraveGeolocationPermission> receiver,
content::RenderFrameHost* rfh) {
auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
if (!web_contents) {
return;
}

auto* tab_helper =
BraveGeolocationPermissionTabHelper::FromWebContents(web_contents);
if (!tab_helper) {
return;
}
tab_helper->brave_geolocation_permission_receivers_.Bind(rfh,
std::move(receiver));
}

void BraveGeolocationPermissionTabHelper::PrimaryPageChanged(
content::Page& page) {
enable_high_accuracy_ = false;
}

void BraveGeolocationPermissionTabHelper::SetEnableHighAccuracy(
bool enable_high_accuracy) {
enable_high_accuracy_ = enable_high_accuracy;
}

WEB_CONTENTS_USER_DATA_KEY_IMPL(BraveGeolocationPermissionTabHelper);
45 changes: 45 additions & 0 deletions browser/ui/geolocation/brave_geolocation_permission_tab_helper.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/* Copyright (c) 2024 The Brave Authors. All rights reserved.
* 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 https://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_BROWSER_UI_GEOLOCATION_BRAVE_GEOLOCATION_PERMISSION_TAB_HELPER_H_
#define BRAVE_BROWSER_UI_GEOLOCATION_BRAVE_GEOLOCATION_PERMISSION_TAB_HELPER_H_

#include "brave/components/brave_geolocation_permission/common/brave_geolocation_permission.mojom.h"
#include "content/public/browser/render_frame_host_receiver_set.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h"

class BraveGeolocationPermissionTabHelper final
: public content::WebContentsObserver,
public geolocation::mojom::BraveGeolocationPermission,
public content::WebContentsUserData<BraveGeolocationPermissionTabHelper> {
public:
explicit BraveGeolocationPermissionTabHelper(content::WebContents* contents);
~BraveGeolocationPermissionTabHelper() override;

static void BindBraveGeolocationPermission(
mojo::PendingAssociatedReceiver<
geolocation::mojom::BraveGeolocationPermission> receiver,
content::RenderFrameHost* rfh);

// content::WebContentsObserver
void PrimaryPageChanged(content::Page& page) override;

// geolocation::mojom::BraveGeolocationPermission overrides:
void SetEnableHighAccuracy(bool high_accuracy) override;

bool enable_high_accuracy() const { return enable_high_accuracy_; }

WEB_CONTENTS_USER_DATA_KEY_DECL();

private:
content::RenderFrameHostReceiverSet<
geolocation::mojom::BraveGeolocationPermission>
brave_geolocation_permission_receivers_;

bool enable_high_accuracy_ = false;
};

#endif // BRAVE_BROWSER_UI_GEOLOCATION_BRAVE_GEOLOCATION_PERMISSION_TAB_HELPER_H_
21 changes: 21 additions & 0 deletions browser/ui/geolocation/geolocation_utils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* Copyright (c) 2024 The Brave Authors. All rights reserved.
* 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 https://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_BROWSER_UI_GEOLOCATION_GEOLOCATION_UTILS_H_
#define BRAVE_BROWSER_UI_GEOLOCATION_GEOLOCATION_UTILS_H_

#include "base/functional/callback_forward.h"

namespace geolocation {

// Run |callback| with true when system location service is available to
// applications.
void IsSystemLocationSettingEnabled(base::OnceCallback<void(bool)> callback);

bool CanGiveDetailedGeolocationRequestInfo();

} // namespace geolocation

#endif // BRAVE_BROWSER_UI_GEOLOCATION_GEOLOCATION_UTILS_H_
18 changes: 18 additions & 0 deletions browser/ui/geolocation/geolocation_utils_linux.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* Copyright (c) 2024 The Brave Authors. All rights reserved.
* 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 https://mozilla.org/MPL/2.0/. */

#include "brave/browser/ui/geolocation/geolocation_utils.h"

#include "base/functional/callback.h"

namespace geolocation {

void IsSystemLocationSettingEnabled(base::OnceCallback<void(bool)> callback) {}

bool CanGiveDetailedGeolocationRequestInfo() {
return false;
}

} // namespace geolocation
49 changes: 49 additions & 0 deletions browser/ui/geolocation/geolocation_utils_mac.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/* Copyright (c) 2024 The Brave Authors. All rights reserved.
* 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 https://mozilla.org/MPL/2.0/. */

#include "brave/browser/ui/geolocation/geolocation_utils.h"

#import <CoreLocation/CoreLocation.h>

#include <utility>

#include "base/functional/callback.h"

namespace geolocation {

namespace {

bool GetSystemLocationSettingEnabled() {
// Service is off globally.
if (![CLLocationManager locationServicesEnabled]) {
return false;
}

if (@available(macOS 11.0, *)) {
CLLocationManager* manager = [[CLLocationManager alloc] init];
if ([manager authorizationStatus] ==
kCLAuthorizationStatusAuthorizedAlways) {
return true;
}
}

return false;
}

} // namespace

void IsSystemLocationSettingEnabled(base::OnceCallback<void(bool)> callback) {
std::move(callback).Run(GetSystemLocationSettingEnabled());
}

bool CanGiveDetailedGeolocationRequestInfo() {
if (@available(macOS 11.0, *)) {
return true;
}

return false;
}

} // namespace geolocation
Loading
Loading