Commit 112eca96 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

permissions: add accessible title to permission prompt bubble

Currently, the permission prompt bubble's title is "$origin wants to:",
which causes a confusing screenreader announcement when the bubble
appears: "Chrome has new window. $origin wants to."

This change introduces a new accessible title, which includes some of
the requested permissions. Specifically, it is one of:

* "$origin wants to: $permission"
* "$origin wants to: $permission and $permission"
* "$origin wants to: $permission, $permission, and more"

To accomplish that, this change:
1) Adds new strings for those messages
2) Adds logic to PermissionPromptBubbleView to build the accessible
   title
3) Slightly refactors PermissionPromptBubbleView to make it possible to
   construct without a live Browser and BrowserWindow, which allows...
4) Adds unit tests for PermissionPromptBubbleView to cover the new
   behavior

Bug: 434574
Change-Id: I3b72f1a51489776c059e952da470034e2e8b131b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2116926
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarAdrienne Porter Felt <felt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755122}
parent 84c207af
...@@ -364,6 +364,15 @@ are declared in tools/grit/grit_rule.gni. ...@@ -364,6 +364,15 @@ are declared in tools/grit/grit_rule.gni.
<message name="IDS_PERMISSIONS_BUBBLE_PROMPT" desc="The label that is used to introduce permission request details to the user in a popup."> <message name="IDS_PERMISSIONS_BUBBLE_PROMPT" desc="The label that is used to introduce permission request details to the user in a popup.">
<ph name="SITE_NAME">$1<ex>google.com</ex></ph> wants to <ph name="SITE_NAME">$1<ex>google.com</ex></ph> wants to
</message> </message>
<message name="IDS_PERMISSIONS_BUBBLE_PROMPT_ACCESSIBLE_TITLE_ONE_PERM" desc="The label that is read by a screen reader to describe the permissions prompt bubble. This will be followed by the short name of a single permission.">
<ph name="SITE_NAME">$1<ex>google.com</ex></ph> wants to: <ph name="PERMISSION">$2<ex>use your microphone</ex></ph>
</message>
<message name="IDS_PERMISSIONS_BUBBLE_PROMPT_ACCESSIBLE_TITLE_TWO_PERMS" desc="The label that is read by a screen reader to describe the permissions prompt bubble. This will be followed by the short names of two permissions.">
<ph name="SITE_NAME">$1<ex>google.com</ex></ph> wants to: <ph name="FIRST_PERMISSION">$2<ex>use your microphone</ex></ph> and <ph name="SECOND_PERMISSION">$3<ex>use your location</ex></ph>
</message>
<message name="IDS_PERMISSIONS_BUBBLE_PROMPT_ACCESSIBLE_TITLE_TWO_PERMS_MORE" desc="The label that is read by a screen reader to describe the permissions prompt bubble. This will be followed by the short names of two permissions, then an &quot;and more&quot; descriptive text indicating that the dialog's body contains the entire list of requested permissions.">
<ph name="SITE_NAME">$1<ex>google.com</ex></ph> wants to: <ph name="FIRST_PERMISSION">$2<ex>use your microphone</ex></ph>, <ph name="SECOND_PERMISSION">$3<ex>use your location</ex></ph>, and more
</message>
<message name="IDS_PERMISSION_CUSTOMIZE" desc="Label on text button to allow the user to customize a permissions request."> <message name="IDS_PERMISSION_CUSTOMIZE" desc="Label on text button to allow the user to customize a permissions request.">
Customize Customize
</message> </message>
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <memory> #include <memory>
#include "base/strings/string_util.h"
#include "chrome/browser/extensions/extension_ui_util.h" #include "chrome/browser/extensions/extension_ui_util.h"
#include "chrome/browser/platform_util.h" #include "chrome/browser/platform_util.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
...@@ -41,7 +42,7 @@ PermissionPromptBubbleView::PermissionPromptBubbleView( ...@@ -41,7 +42,7 @@ PermissionPromptBubbleView::PermissionPromptBubbleView(
: browser_(browser), : browser_(browser),
delegate_(delegate), delegate_(delegate),
name_or_origin_(GetDisplayNameOrOrigin()) { name_or_origin_(GetDisplayNameOrOrigin()) {
DCHECK(browser_); // Note that browser_ may be null in unit tests.
DCHECK(delegate_); DCHECK(delegate_);
// To prevent permissions being accepted accidentally, and as a security // To prevent permissions being accepted accidentally, and as a security
...@@ -72,9 +73,25 @@ PermissionPromptBubbleView::PermissionPromptBubbleView( ...@@ -72,9 +73,25 @@ PermissionPromptBubbleView::PermissionPromptBubbleView(
for (permissions::PermissionRequest* request : delegate_->Requests()) for (permissions::PermissionRequest* request : delegate_->Requests())
AddPermissionRequestLine(request); AddPermissionRequestLine(request);
}
void PermissionPromptBubbleView::Show() {
DCHECK(browser_->window());
Show(); // Set |parent_window| because some valid anchors can become hidden.
set_parent_window(
platform_util::GetViewForWindow(browser_->window()->GetNativeWindow()));
views::Widget* widget = views::BubbleDialogDelegateView::CreateBubble(this);
// If a browser window (or popup) other than the bubble parent has focus,
// don't take focus.
if (browser_->window()->IsActive())
widget->Show();
else
widget->ShowInactive();
SizeToContents();
UpdateAnchorPosition();
chrome::RecordDialogCreation(chrome::DialogIdentifier::PERMISSIONS); chrome::RecordDialogCreation(chrome::DialogIdentifier::PERMISSIONS);
} }
...@@ -172,6 +189,38 @@ base::string16 PermissionPromptBubbleView::GetWindowTitle() const { ...@@ -172,6 +189,38 @@ base::string16 PermissionPromptBubbleView::GetWindowTitle() const {
name_or_origin_.name_or_origin); name_or_origin_.name_or_origin);
} }
base::string16 PermissionPromptBubbleView::GetAccessibleWindowTitle() const {
// Generate one of:
// $origin wants to: $permission
// $origin wants to: $permission and $permission
// $origin wants to: $permission, $permission, and more
// where $permission is the permission's text fragment, a verb phrase
// describing what the permission is, like:
// "Download multiple files"
// "Use your camera"
//
// There are three separate internationalized messages used, one for each
// format of title, to provide for accurate i18n. See https://crbug.com/434574
// for more details.
const std::vector<permissions::PermissionRequest*>& requests =
delegate_->Requests();
DCHECK(!requests.empty());
if (requests.size() == 1) {
return l10n_util::GetStringFUTF16(
IDS_PERMISSIONS_BUBBLE_PROMPT_ACCESSIBLE_TITLE_ONE_PERM,
name_or_origin_.name_or_origin, requests[0]->GetMessageTextFragment());
}
int template_id =
requests.size() == 2
? IDS_PERMISSIONS_BUBBLE_PROMPT_ACCESSIBLE_TITLE_TWO_PERMS
: IDS_PERMISSIONS_BUBBLE_PROMPT_ACCESSIBLE_TITLE_TWO_PERMS_MORE;
return l10n_util::GetStringFUTF16(template_id, name_or_origin_.name_or_origin,
requests[0]->GetMessageTextFragment(),
requests[1]->GetMessageTextFragment());
}
gfx::Size PermissionPromptBubbleView::CalculatePreferredSize() const { gfx::Size PermissionPromptBubbleView::CalculatePreferredSize() const {
const int width = ChromeLayoutProvider::Get()->GetDistanceMetric( const int width = ChromeLayoutProvider::Get()->GetDistanceMetric(
DISTANCE_BUBBLE_PREFERRED_WIDTH) - DISTANCE_BUBBLE_PREFERRED_WIDTH) -
...@@ -187,25 +236,6 @@ void PermissionPromptBubbleView::ButtonPressed(views::Button* sender, ...@@ -187,25 +236,6 @@ void PermissionPromptBubbleView::ButtonPressed(views::Button* sender,
ui::PAGE_TRANSITION_LINK); ui::PAGE_TRANSITION_LINK);
} }
void PermissionPromptBubbleView::Show() {
DCHECK(browser_->window());
// Set |parent_window| because some valid anchors can become hidden.
set_parent_window(
platform_util::GetViewForWindow(browser_->window()->GetNativeWindow()));
views::Widget* widget = views::BubbleDialogDelegateView::CreateBubble(this);
// If a browser window (or popup) other than the bubble parent has focus,
// don't take focus.
if (browser_->window()->IsActive())
widget->Show();
else
widget->ShowInactive();
SizeToContents();
UpdateAnchorPosition();
}
PermissionPromptBubbleView::DisplayNameOrOrigin PermissionPromptBubbleView::DisplayNameOrOrigin
PermissionPromptBubbleView::GetDisplayNameOrOrigin() { PermissionPromptBubbleView::GetDisplayNameOrOrigin() {
const std::vector<permissions::PermissionRequest*>& requests = const std::vector<permissions::PermissionRequest*>& requests =
......
...@@ -25,6 +25,8 @@ class PermissionPromptBubbleView : public views::ButtonListener, ...@@ -25,6 +25,8 @@ class PermissionPromptBubbleView : public views::ButtonListener,
PermissionPromptBubbleView(Browser* browser, PermissionPromptBubbleView(Browser* browser,
permissions::PermissionPrompt::Delegate* delegate); permissions::PermissionPrompt::Delegate* delegate);
void Show();
// Anchors the bubble to the view or rectangle returned from // Anchors the bubble to the view or rectangle returned from
// bubble_anchor_util::GetPageInfoAnchorConfiguration. // bubble_anchor_util::GetPageInfoAnchorConfiguration.
void UpdateAnchorPosition(); void UpdateAnchorPosition();
...@@ -32,6 +34,7 @@ class PermissionPromptBubbleView : public views::ButtonListener, ...@@ -32,6 +34,7 @@ class PermissionPromptBubbleView : public views::ButtonListener,
// views::BubbleDialogDelegateView: // views::BubbleDialogDelegateView:
void AddedToWidget() override; void AddedToWidget() override;
bool ShouldShowCloseButton() const override; bool ShouldShowCloseButton() const override;
base::string16 GetAccessibleWindowTitle() const override;
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
gfx::Size CalculatePreferredSize() const override; gfx::Size CalculatePreferredSize() const override;
...@@ -48,8 +51,6 @@ class PermissionPromptBubbleView : public views::ButtonListener, ...@@ -48,8 +51,6 @@ class PermissionPromptBubbleView : public views::ButtonListener,
void AddPermissionRequestLine(permissions::PermissionRequest* request); void AddPermissionRequestLine(permissions::PermissionRequest* request);
void Show();
// Returns the origin to be displayed in the permission prompt. May return // Returns the origin to be displayed in the permission prompt. May return
// a non-origin, e.g. extension URLs use the name of the extension. // a non-origin, e.g. extension URLs use the name of the extension.
DisplayNameOrOrigin GetDisplayNameOrOrigin(); DisplayNameOrOrigin GetDisplayNameOrOrigin();
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.h"
#include "chrome/test/views/chrome_views_test_base.h"
#include "components/permissions/test/mock_permission_request.h"
using PermissionPromptBubbleViewTest = ChromeViewsTestBase;
namespace {
class TestDelegate : public permissions::PermissionPrompt::Delegate {
public:
TestDelegate(const GURL& origin, const std::vector<std::string> names) {
std::transform(
names.begin(), names.end(), std::back_inserter(requests_),
[&](auto& name) {
return std::make_unique<permissions::MockPermissionRequest>(
name, permissions::PermissionRequestType::UNKNOWN, origin);
});
std::transform(requests_.begin(), requests_.end(),
std::back_inserter(raw_requests_),
[](auto& req) { return req.get(); });
}
const std::vector<permissions::PermissionRequest*>& Requests() override {
return raw_requests_;
}
void Accept() override {}
void Deny() override {}
void Closing() override {}
private:
std::vector<std::unique_ptr<permissions::PermissionRequest>> requests_;
std::vector<permissions::PermissionRequest*> raw_requests_;
};
TEST_F(PermissionPromptBubbleViewTest, AccessibleTitleMentionsPermissions) {
TestDelegate delegate(GURL("https://test.origin"), {"foo", "bar"});
auto bubble =
std::make_unique<PermissionPromptBubbleView>(nullptr, &delegate);
EXPECT_PRED_FORMAT2(::testing::IsSubstring, "foo",
base::UTF16ToUTF8(bubble->GetAccessibleWindowTitle()));
EXPECT_PRED_FORMAT2(::testing::IsSubstring, "bar",
base::UTF16ToUTF8(bubble->GetAccessibleWindowTitle()));
}
TEST_F(PermissionPromptBubbleViewTest, AccessibleTitleMentionsOrigin) {
TestDelegate delegate(GURL("https://test.origin"), {"foo", "bar"});
auto bubble =
std::make_unique<PermissionPromptBubbleView>(nullptr, &delegate);
// Note that the scheme is not usually included.
EXPECT_PRED_FORMAT2(::testing::IsSubstring, "test.origin",
base::UTF16ToUTF8(bubble->GetAccessibleWindowTitle()));
}
TEST_F(PermissionPromptBubbleViewTest,
AccessibleTitleDoesNotMentionTooManyPermissions) {
TestDelegate delegate(GURL("https://test.origin"),
{"foo", "bar", "baz", "quxx"});
auto bubble =
std::make_unique<PermissionPromptBubbleView>(nullptr, &delegate);
const auto title = base::UTF16ToUTF8(bubble->GetAccessibleWindowTitle());
EXPECT_PRED_FORMAT2(::testing::IsSubstring, "foo", title);
EXPECT_PRED_FORMAT2(::testing::IsSubstring, "bar", title);
EXPECT_PRED_FORMAT2(::testing::IsNotSubstring, "baz", title);
EXPECT_PRED_FORMAT2(::testing::IsNotSubstring, "quxx", title);
}
} // namespace
...@@ -42,6 +42,7 @@ PermissionPromptImpl::PermissionPromptImpl(Browser* browser, ...@@ -42,6 +42,7 @@ PermissionPromptImpl::PermissionPromptImpl(Browser* browser,
content_settings::UpdateLocationBarUiForWebContents(web_contents_); content_settings::UpdateLocationBarUiForWebContents(web_contents_);
} else { } else {
prompt_bubble_ = new PermissionPromptBubbleView(browser, delegate); prompt_bubble_ = new PermissionPromptBubbleView(browser, delegate);
prompt_bubble_->Show();
} }
} }
......
...@@ -5333,6 +5333,7 @@ test("unit_tests") { ...@@ -5333,6 +5333,7 @@ test("unit_tests") {
"../browser/ui/views/payments/payment_request_item_list_unittest.cc", "../browser/ui/views/payments/payment_request_item_list_unittest.cc",
"../browser/ui/views/payments/validating_textfield_unittest.cc", "../browser/ui/views/payments/validating_textfield_unittest.cc",
"../browser/ui/views/payments/view_stack_unittest.cc", "../browser/ui/views/payments/view_stack_unittest.cc",
"../browser/ui/views/permission_bubble/permission_prompt_bubble_view_unittest.cc",
"../browser/ui/views/profiles/avatar_toolbar_button_unittest.cc", "../browser/ui/views/profiles/avatar_toolbar_button_unittest.cc",
"../browser/ui/views/relaunch_notification/relaunch_notification_controller_unittest.cc", "../browser/ui/views/relaunch_notification/relaunch_notification_controller_unittest.cc",
"../browser/ui/views/relaunch_notification/relaunch_required_timer_internal_unittest.cc", "../browser/ui/views/relaunch_notification/relaunch_required_timer_internal_unittest.cc",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment