Commit 82504ea9 authored by Qiang Xu's avatar Qiang Xu Committed by Commit Bot

cros: move ShowAccessibilityNotification to A11yController

changes:
Remove ShowAccessibilityNotification in AccessibilityObserver. That is
only used in TrayAccessibility, while TrayAccessibility should be pure
views code. Move the function and tests to AccessibilityController.

Bug: 800270
Test: covered by tests
Change-Id: I0551c8b36d1a4bfb030b786bd78d610627113828
Reviewed-on: https://chromium-review.googlesource.com/1007891Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Qiang Xu <warx@google.com>
Cr-Commit-Position: refs/heads/master@{#551253}
parent f856991a
......@@ -16,13 +16,16 @@
#include "ash/public/cpp/ash_pref_names.h"
#include "ash/public/cpp/config.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/session/session_controller.h"
#include "ash/session/session_observer.h"
#include "ash/shell.h"
#include "ash/shell_port.h"
#include "ash/sticky_keys/sticky_keys_controller.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/system/power/backlights_forced_off_setter.h"
#include "ash/system/power/scoped_backlights_forced_off.h"
#include "base/strings/string16.h"
#include "chromeos/audio/cras_audio_handler.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_change_registrar.h"
......@@ -34,13 +37,19 @@
#include "services/ui/public/interfaces/constants.mojom.h"
#include "ui/aura/window.h"
#include "ui/base/cursor/cursor_type.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/keyboard/keyboard_util.h"
#include "ui/message_center/message_center.h"
#include "ui/message_center/public/cpp/notifier_id.h"
using session_manager::SessionState;
namespace ash {
namespace {
constexpr char kNotificationId[] = "chrome://settings/accessibility";
constexpr char kNotifierAccessibility[] = "ash.accessibility";
// List of accessibility prefs that are to be copied (if changed by the user) on
// signin screen profile to a newly created user profile or a guest session.
constexpr const char* const kCopiedOnSigninAccessibilityPrefs[]{
......@@ -131,6 +140,68 @@ void CopySigninPrefsIfNeeded(PrefService* previous_pref_service,
}
}
// Used to indicate which accessibility notification should be shown.
enum class A11yNotificationType {
// No accessibility notification.
kNone,
// Shown when spoken feedback is set enabled with A11Y_NOTIFICATION_SHOW.
kSpokenFeedbackEnabled,
// Shown when braille display is connected while spoken feedback is enabled.
kBrailleDisplayConnected,
// Shown when braille display is connected while spoken feedback is not
// enabled yet. Note: in this case braille display connected would enable
// spoken feeback.
kSpokenFeedbackBrailleEnabled,
};
// Returns notification icon based on the A11yNotificationType.
const gfx::VectorIcon& GetNotificationIcon(A11yNotificationType type) {
if (type == A11yNotificationType::kSpokenFeedbackBrailleEnabled)
return kNotificationAccessibilityIcon;
if (type == A11yNotificationType::kBrailleDisplayConnected)
return kNotificationAccessibilityBrailleIcon;
return kNotificationChromevoxIcon;
}
void ShowAccessibilityNotification(A11yNotificationType type) {
message_center::MessageCenter* message_center =
message_center::MessageCenter::Get();
message_center->RemoveNotification(kNotificationId, false /* by_user */);
if (type == A11yNotificationType::kNone)
return;
base::string16 text;
base::string16 title;
if (type == A11yNotificationType::kSpokenFeedbackBrailleEnabled) {
text =
l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_SPOKEN_FEEDBACK_ENABLED);
title = l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_SPOKEN_FEEDBACK_BRAILLE_ENABLED_TITLE);
} else if (type == A11yNotificationType::kBrailleDisplayConnected) {
text = l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_BRAILLE_DISPLAY_CONNECTED);
} else {
title = l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_SPOKEN_FEEDBACK_ENABLED_TITLE);
text =
l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_SPOKEN_FEEDBACK_ENABLED);
}
message_center::RichNotificationData options;
options.should_make_spoken_feedback_for_popup_updates = false;
std::unique_ptr<message_center::Notification> notification =
message_center::Notification::CreateSystemNotification(
message_center::NOTIFICATION_TYPE_SIMPLE, kNotificationId, title,
text, gfx::Image(), base::string16(), GURL(),
message_center::NotifierId(
message_center::NotifierId::SYSTEM_COMPONENT,
kNotifierAccessibility),
options, nullptr, GetNotificationIcon(type),
message_center::SystemNotificationWarningLevel::NORMAL);
notification->set_priority(message_center::SYSTEM_PRIORITY);
message_center->AddNotification(std::move(notification));
}
} // namespace
AccessibilityController::AccessibilityController(
......@@ -303,10 +374,14 @@ void AccessibilityController::SetSpokenFeedbackEnabled(
AccessibilityNotificationVisibility notify) {
if (!active_user_prefs_)
return;
spoken_feedback_notification_ = notify;
active_user_prefs_->SetBoolean(prefs::kAccessibilitySpokenFeedbackEnabled,
enabled);
active_user_prefs_->CommitPendingWrite();
A11yNotificationType type = A11yNotificationType::kNone;
if (enabled && notify == A11Y_NOTIFICATION_SHOW)
type = A11yNotificationType::kSpokenFeedbackEnabled;
ShowAccessibilityNotification(type);
}
bool AccessibilityController::IsSpokenFeedbackEnabled() const {
......@@ -424,11 +499,18 @@ void AccessibilityController::SetDarkenScreen(bool darken) {
}
void AccessibilityController::BrailleDisplayStateChanged(bool connected) {
A11yNotificationType type = A11yNotificationType::kNone;
if (connected && spoken_feedback_enabled_)
type = A11yNotificationType::kBrailleDisplayConnected;
else if (connected && !spoken_feedback_enabled_)
type = A11yNotificationType::kSpokenFeedbackBrailleEnabled;
braille_display_connected_ = connected;
if (braille_display_connected_)
SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_SHOW);
SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_NONE);
NotifyAccessibilityStatusChanged();
NotifyShowAccessibilityNotification();
ShowAccessibilityNotification(type);
}
void AccessibilityController::SetFocusHighlightRect(
......@@ -720,9 +802,6 @@ void AccessibilityController::UpdateSpokenFeedbackFromPref() {
NotifyAccessibilityStatusChanged();
if (spoken_feedback_notification_ != A11Y_NOTIFICATION_NONE)
NotifyShowAccessibilityNotification();
// TODO(warx): ChromeVox loading/unloading requires browser process started,
// thus it is still handled on Chrome side.
......@@ -808,9 +887,4 @@ void AccessibilityController::UpdateVirtualKeyboardFromPref() {
Shell::Get()->DestroyKeyboard();
}
void AccessibilityController::NotifyShowAccessibilityNotification() {
for (auto& observer : observers_)
observer.ShowAccessibilityNotification();
}
} // namespace ash
......@@ -167,8 +167,6 @@ class ASH_EXPORT AccessibilityController
void UpdateVirtualKeyboardFromPref();
void UpdateAccessibilityHighlightingFromPrefs();
void NotifyShowAccessibilityNotification();
service_manager::Connector* connector_ = nullptr;
// The pref service of the currently active user or the signin profile before
......@@ -198,11 +196,6 @@ class ASH_EXPORT AccessibilityController
bool virtual_keyboard_enabled_ = false;
bool braille_display_connected_ = false;
// TODO(warx): consider removing this and replacing it with a more reliable
// way (https://crbug.com/800270).
AccessibilityNotificationVisibility spoken_feedback_notification_ =
A11Y_NOTIFICATION_NONE;
// Used to control the highlights of caret, cursor and focus.
std::unique_ptr<AccessibilityHighlightController>
accessibility_highlight_controller_;
......
......@@ -4,6 +4,8 @@
#include "ash/accessibility/accessibility_controller.h"
#include <utility>
#include "ash/accessibility/accessibility_observer.h"
#include "ash/accessibility/test_accessibility_controller_client.h"
#include "ash/ash_constants.h"
......@@ -15,11 +17,17 @@
#include "ash/shell.h"
#include "ash/sticky_keys/sticky_keys_controller.h"
#include "ash/test/ash_test_base.h"
#include "base/macros.h"
#include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/fake_power_manager_client.h"
#include "components/prefs/pref_service.h"
#include "ui/keyboard/keyboard_util.h"
#include "ui/message_center/message_center.h"
using message_center::MessageCenter;
namespace ash {
......@@ -358,6 +366,79 @@ TEST_F(AccessibilityControllerTest, SetDarkenScreen) {
EXPECT_FALSE(power_manager_client_->backlights_forced_off());
}
TEST_F(AccessibilityControllerTest, ShowNotificationOnSpokenFeedback) {
const base::string16 kChromeVoxEnabledTitle =
base::ASCIIToUTF16("ChromeVox enabled");
const base::string16 kChromeVoxEnabled =
base::ASCIIToUTF16("Press Ctrl + Alt + Z to disable spoken feedback.");
AccessibilityController* controller =
Shell::Get()->accessibility_controller();
// Enabling spoken feedback should show the notification if specified to show
// notification.
controller->SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_SHOW);
message_center::NotificationList::Notifications notifications =
MessageCenter::Get()->GetVisibleNotifications();
ASSERT_EQ(1u, notifications.size());
EXPECT_EQ(kChromeVoxEnabledTitle, (*notifications.begin())->title());
EXPECT_EQ(kChromeVoxEnabled, (*notifications.begin())->message());
// Disabling spoken feedback should not show any notification even if
// specified to show notification.
controller->SetSpokenFeedbackEnabled(false, A11Y_NOTIFICATION_SHOW);
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(0u, notifications.size());
// Enabling spoken feedback but not specified to show notification should not
// show any notification, for example toggling on tray detailed menu.
controller->SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_NONE);
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(0u, notifications.size());
}
TEST_F(AccessibilityControllerTest,
ShowNotificationOnBrailleDisplayStateChanged) {
const base::string16 kBrailleConnected =
base::ASCIIToUTF16("Braille display connected.");
const base::string16 kChromeVoxEnabled =
base::ASCIIToUTF16("Press Ctrl + Alt + Z to disable spoken feedback.");
const base::string16 kBrailleConnectedAndChromeVoxEnabledTitle =
base::ASCIIToUTF16("Braille and ChromeVox are enabled");
AccessibilityController* controller =
Shell::Get()->accessibility_controller();
controller->SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_SHOW);
EXPECT_TRUE(controller->IsSpokenFeedbackEnabled());
// Connecting a braille display when spoken feedback is already enabled
// should only show the message about the braille display.
controller->BrailleDisplayStateChanged(true);
message_center::NotificationList::Notifications notifications =
MessageCenter::Get()->GetVisibleNotifications();
ASSERT_EQ(1u, notifications.size());
EXPECT_EQ(base::string16(), (*notifications.begin())->title());
EXPECT_EQ(kBrailleConnected, (*notifications.begin())->message());
// Neither disconnecting a braille display, nor disabling spoken feedback
// should show any notification.
controller->BrailleDisplayStateChanged(false);
EXPECT_TRUE(controller->IsSpokenFeedbackEnabled());
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(0u, notifications.size());
controller->SetSpokenFeedbackEnabled(false, A11Y_NOTIFICATION_SHOW);
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(0u, notifications.size());
EXPECT_FALSE(controller->IsSpokenFeedbackEnabled());
// Connecting a braille display should enable spoken feedback and show
// both messages.
controller->BrailleDisplayStateChanged(true);
EXPECT_TRUE(controller->IsSpokenFeedbackEnabled());
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(kBrailleConnectedAndChromeVoxEnabledTitle,
(*notifications.begin())->title());
EXPECT_EQ(kChromeVoxEnabled, (*notifications.begin())->message());
}
namespace {
enum class TestUserLoginType {
......
......@@ -11,15 +11,10 @@ namespace ash {
class ASH_EXPORT AccessibilityObserver {
public:
virtual ~AccessibilityObserver() {}
virtual ~AccessibilityObserver() = default;
// Called when any accessibility status changes.
virtual void OnAccessibilityStatusChanged() {}
// Called when notification should be shown for accessibility status changes,
// used for spoken feedback or braille is enabled.
// TODO(warx): move this to AccessibilityController.
virtual void ShowAccessibilityNotification() {}
virtual void OnAccessibilityStatusChanged() = 0;
};
} // namespace ash
......
......@@ -30,8 +30,6 @@
#include "ui/gfx/image/image.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/gfx/vector_icon_types.h"
#include "ui/message_center/message_center.h"
#include "ui/message_center/public/cpp/notifier_id.h"
#include "ui/native_theme/native_theme.h"
#include "ui/views/controls/separator.h"
#include "ui/views/widget/widget.h"
......@@ -39,9 +37,6 @@
namespace ash {
namespace {
const char kNotificationId[] = "chrome://settings/accessibility";
const char kNotifierAccessibility[] = "ash.accessibility";
enum AccessibilityState {
A11Y_NONE = 0,
A11Y_SPOKEN_FEEDBACK = 1 << 0,
......@@ -102,17 +97,6 @@ LoginStatus GetCurrentLoginStatus() {
return Shell::Get()->session_controller()->login_status();
}
// Returns notification icon based on the enabled accessibility state.
const gfx::VectorIcon& GetNotificationIcon(uint32_t enabled_accessibility) {
if ((enabled_accessibility & A11Y_BRAILLE_DISPLAY_CONNECTED) &&
(enabled_accessibility & A11Y_SPOKEN_FEEDBACK)) {
return kNotificationAccessibilityIcon;
}
if (enabled_accessibility & A11Y_BRAILLE_DISPLAY_CONNECTED)
return kNotificationAccessibilityBrailleIcon;
return kNotificationChromevoxIcon;
}
} // namespace
namespace tray {
......@@ -457,7 +441,6 @@ TrayAccessibility::TrayAccessibility(SystemTray* system_tray)
detailed_menu_(nullptr),
tray_icon_visible_(false),
login_(GetCurrentLoginStatus()),
previous_accessibility_state_(GetAccessibilityState()),
show_a11y_menu_on_lock_screen_(true) {
DCHECK(system_tray);
Shell::Get()->accessibility_controller()->AddObserver(this);
......@@ -537,61 +520,4 @@ void TrayAccessibility::OnAccessibilityStatusChanged() {
detailed_menu_->OnAccessibilityStatusChanged();
}
// TODO(warx): Move ShowAccessibilityNotification() to AccessibilityController.
void TrayAccessibility::ShowAccessibilityNotification() {
uint32_t accessibility_state = GetAccessibilityState();
// We'll get an extra notification if a braille display is connected when
// spoken feedback wasn't already enabled. This is because the braille
// connection state is already updated when spoken feedback is enabled so
// that the notifications can be consolidated into one. Therefore, we
// return early if there's no change in the state that we keep track of.
if (accessibility_state == previous_accessibility_state_)
return;
message_center::MessageCenter* message_center =
message_center::MessageCenter::Get();
message_center->RemoveNotification(kNotificationId, false /* by_user */);
// Contains bits for spoken feedback and braille display connected currently
// being enabled.
uint32_t being_enabled =
(accessibility_state & ~previous_accessibility_state_) &
(A11Y_SPOKEN_FEEDBACK | A11Y_BRAILLE_DISPLAY_CONNECTED);
previous_accessibility_state_ = accessibility_state;
if (being_enabled == A11Y_NONE)
return;
base::string16 text;
base::string16 title;
if (being_enabled & A11Y_BRAILLE_DISPLAY_CONNECTED &&
being_enabled & A11Y_SPOKEN_FEEDBACK) {
text =
l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_SPOKEN_FEEDBACK_ENABLED);
title = l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_SPOKEN_FEEDBACK_BRAILLE_ENABLED_TITLE);
} else if (being_enabled & A11Y_BRAILLE_DISPLAY_CONNECTED) {
text = l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_BRAILLE_DISPLAY_CONNECTED);
} else {
title = l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_SPOKEN_FEEDBACK_ENABLED_TITLE);
text =
l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_SPOKEN_FEEDBACK_ENABLED);
}
message_center::RichNotificationData options;
options.should_make_spoken_feedback_for_popup_updates = false;
std::unique_ptr<message_center::Notification> notification =
message_center::Notification::CreateSystemNotification(
message_center::NOTIFICATION_TYPE_SIMPLE, kNotificationId, title,
text, gfx::Image(), base::string16(), GURL(),
message_center::NotifierId(
message_center::NotifierId::SYSTEM_COMPONENT,
kNotifierAccessibility),
options, nullptr, GetNotificationIcon(being_enabled),
message_center::SystemNotificationWarningLevel::NORMAL);
notification->set_priority(message_center::SYSTEM_PRIORITY);
message_center->AddNotification(std::move(notification));
}
} // namespace ash
......@@ -123,7 +123,6 @@ class TrayAccessibility : public TrayImageItem, public AccessibilityObserver {
// Overridden from AccessibilityObserver.
void OnAccessibilityStatusChanged() override;
void ShowAccessibilityNotification() override;
views::View* default_;
tray::AccessibilityDetailedView* detailed_menu_;
......@@ -131,9 +130,6 @@ class TrayAccessibility : public TrayImageItem, public AccessibilityObserver {
bool tray_icon_visible_;
LoginStatus login_;
// Bitmap of values from AccessibilityState enum.
uint32_t previous_accessibility_state_;
// A11y feature status on just entering the lock screen.
bool show_a11y_menu_on_lock_screen_;
......
......@@ -12,12 +12,7 @@
#include "ash/system/tray/system_tray_test_api.h"
#include "ash/test/ash_test_base.h"
#include "base/macros.h"
#include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h"
#include "components/prefs/pref_service.h"
#include "ui/message_center/message_center.h"
using message_center::MessageCenter;
namespace ash {
namespace {
......@@ -157,79 +152,6 @@ TEST_F(TrayAccessibilityTest, VisibilityFromSettings) {
EXPECT_FALSE(tray_item_->tray_view()->visible());
}
TEST_F(TrayAccessibilityTest, ShowNotificationOnSpokenFeedback) {
const base::string16 kChromeVoxEnabledTitle =
base::ASCIIToUTF16("ChromeVox enabled");
const base::string16 kChromeVoxEnabled =
base::ASCIIToUTF16("Press Ctrl + Alt + Z to disable spoken feedback.");
AccessibilityController* controller =
Shell::Get()->accessibility_controller();
// Enabling spoken feedback should show the notification if specified to show
// notification.
controller->SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_SHOW);
message_center::NotificationList::Notifications notifications =
MessageCenter::Get()->GetVisibleNotifications();
ASSERT_EQ(1u, notifications.size());
EXPECT_EQ(kChromeVoxEnabledTitle, (*notifications.begin())->title());
EXPECT_EQ(kChromeVoxEnabled, (*notifications.begin())->message());
// Disabling spoken feedback should not show any notification even if
// specified to show notification.
controller->SetSpokenFeedbackEnabled(false, A11Y_NOTIFICATION_SHOW);
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(0u, notifications.size());
// Enabling spoken feedback but not specified to show notification should not
// show any notification, for example toggling on tray detailed menu.
// TODO(warx): migrate clicking on tray detailed menu from browser tests.
controller->SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_NONE);
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(0u, notifications.size());
}
TEST_F(TrayAccessibilityTest, ShowNotificationOnBrailleDisplayStateChanged) {
const base::string16 kBrailleConnected =
base::ASCIIToUTF16("Braille display connected.");
const base::string16 kChromeVoxEnabled =
base::ASCIIToUTF16("Press Ctrl + Alt + Z to disable spoken feedback.");
const base::string16 kBrailleConnectedAndChromeVoxEnabledTitle =
base::ASCIIToUTF16("Braille and ChromeVox are enabled");
AccessibilityController* controller =
Shell::Get()->accessibility_controller();
controller->SetSpokenFeedbackEnabled(true, A11Y_NOTIFICATION_SHOW);
EXPECT_TRUE(controller->IsSpokenFeedbackEnabled());
// Connecting a braille display when spoken feedback is already enabled
// should only show the message about the braille display.
controller->BrailleDisplayStateChanged(true);
message_center::NotificationList::Notifications notifications =
MessageCenter::Get()->GetVisibleNotifications();
ASSERT_EQ(1u, notifications.size());
EXPECT_EQ(base::string16(), (*notifications.begin())->title());
EXPECT_EQ(kBrailleConnected, (*notifications.begin())->message());
// Neither disconnecting a braille display, nor disabling spoken feedback
// should show any notification.
controller->BrailleDisplayStateChanged(false);
EXPECT_TRUE(controller->IsSpokenFeedbackEnabled());
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(0u, notifications.size());
controller->SetSpokenFeedbackEnabled(false, A11Y_NOTIFICATION_SHOW);
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(0u, notifications.size());
EXPECT_FALSE(controller->IsSpokenFeedbackEnabled());
// Connecting a braille display should enable spoken feedback and show
// both messages.
controller->BrailleDisplayStateChanged(true);
EXPECT_TRUE(controller->IsSpokenFeedbackEnabled());
notifications = MessageCenter::Get()->GetVisibleNotifications();
EXPECT_EQ(kBrailleConnectedAndChromeVoxEnabledTitle,
(*notifications.begin())->title());
EXPECT_EQ(kChromeVoxEnabled, (*notifications.begin())->message());
}
TEST_F(TrayAccessibilityTest, CheckMenuVisibilityOnDetailMenu) {
// Except help & settings, others should be kept the same
// in LOGIN | NOT LOGIN | LOCKED. https://crbug.com/632107.
......
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