[Read Anything] Implement Prefs for read anything user settings
With this CL we add an implementation to save user settings of
the read anything feature to Prefs.
The Read Anything feature will include options to change font
family, size, spacing, etc, and we want these settings to
persist across sessions. We also want these to be included in
the Profile so that a user's accessibility settings will work
across machines. With this CL we add the first such preference
to the Profile, for the font type choice.
AX-Relnotes: N/A
Bug: 1266555
Change-Id: Id35903bd80261ed825b11e9bc29f42059ab9900b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3696841
Reviewed-by: David Tseng <[email protected]>
Reviewed-by: Yaron Friedman <[email protected]>
Commit-Queue: Mark Schillaci <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1019151}
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 1b759a6..f93e220 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -266,6 +266,7 @@
#include "chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.h"
#include "chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.h"
#include "chrome/browser/ui/webui/settings/settings_ui.h"
+#include "chrome/browser/ui/webui/side_panel/read_anything/read_anything_prefs.h"
#include "chrome/browser/ui/webui/tab_search/tab_search_prefs.h"
#include "chrome/browser/ui/webui/whats_new/whats_new_ui.h"
#include "chrome/browser/upgrade_detector/upgrade_detector.h"
@@ -1420,6 +1421,7 @@
PhotosService::RegisterProfilePrefs(registry);
PinnedTabCodec::RegisterProfilePrefs(registry);
PromoService::RegisterProfilePrefs(registry);
+ RegisterReadAnythingProfilePrefs(registry);
settings::SettingsUI::RegisterProfilePrefs(registry);
send_tab_to_self::SendTabToSelfBubbleController::RegisterProfilePrefs(
registry);
diff --git a/chrome/browser/resources/side_panel/read_anything/app.ts b/chrome/browser/resources/side_panel/read_anything/app.ts
index 67aec2c..c0e2519 100644
--- a/chrome/browser/resources/side_panel/read_anything/app.ts
+++ b/chrome/browser/resources/side_panel/read_anything/app.ts
@@ -58,7 +58,7 @@
// the names set in read_anything_font_model.cc.
private defaultFontName: string = 'standard';
private validFontNames: {name: string, cssClass: string}[] = [
- {name: 'Standard', cssClass: 'standard'},
+ {name: 'Standard font', cssClass: 'standard'},
{name: 'Serif', cssClass: 'serif'},
{name: 'Sans-serif', cssClass: 'sans-serif'},
{name: 'Arial', cssClass: 'arial'},
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index b13d291d..6319024 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -1674,6 +1674,8 @@
"webui/side_panel/history_clusters/history_clusters_side_panel_ui.h",
"webui/side_panel/read_anything/read_anything_page_handler.cc",
"webui/side_panel/read_anything/read_anything_page_handler.h",
+ "webui/side_panel/read_anything/read_anything_prefs.cc",
+ "webui/side_panel/read_anything/read_anything_prefs.h",
"webui/side_panel/read_anything/read_anything_ui.cc",
"webui/side_panel/read_anything/read_anything_ui.h",
"webui/side_panel/reading_list/reading_list_page_handler.cc",
diff --git a/chrome/browser/ui/views/side_panel/read_anything/read_anything_constants.h b/chrome/browser/ui/views/side_panel/read_anything/read_anything_constants.h
index 65e3eea..7e682143 100644
--- a/chrome/browser/ui/views/side_panel/read_anything/read_anything_constants.h
+++ b/chrome/browser/ui/views/side_panel/read_anything/read_anything_constants.h
@@ -15,6 +15,8 @@
const int kIconSize = 16;
const int kIconCornerRadius = 2;
+const char kReadAnythingDefaultFontName[] = "Standard font";
+
} // namespace
#endif // CHROME_BROWSER_UI_VIEWS_SIDE_PANEL_READ_ANYTHING_READ_ANYTHING_CONSTANTS_H_
diff --git a/chrome/browser/ui/views/side_panel/read_anything/read_anything_controller.cc b/chrome/browser/ui/views/side_panel/read_anything/read_anything_controller.cc
index c46e9bee..a267f9ff 100644
--- a/chrome/browser/ui/views/side_panel/read_anything/read_anything_controller.cc
+++ b/chrome/browser/ui/views/side_panel/read_anything/read_anything_controller.cc
@@ -7,6 +7,7 @@
#include <vector>
#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/webui/side_panel/read_anything/read_anything_prefs.h"
#include "chrome/common/accessibility/read_anything.mojom.h"
#include "ui/accessibility/ax_tree_update.h"
@@ -31,7 +32,12 @@
}
void ReadAnythingController::OnFontChoiceChanged(int new_choice) {
- model_->SetSelectedFontIndex(new_choice);
+ std::string new_font_name;
+ model_->SetSelectedFontByIndex(new_choice);
+
+ browser_->profile()->GetPrefs()->SetString(
+ prefs::kAccessibilityReadAnythingFontName,
+ model_->GetFontModel()->GetFontNameAt(new_choice));
}
void ReadAnythingController::OnUIReady() {
diff --git a/chrome/browser/ui/views/side_panel/read_anything/read_anything_coordinator.cc b/chrome/browser/ui/views/side_panel/read_anything/read_anything_coordinator.cc
index 5eb85dc4..4ea28c9 100644
--- a/chrome/browser/ui/views/side_panel/read_anything/read_anything_coordinator.cc
+++ b/chrome/browser/ui/views/side_panel/read_anything/read_anything_coordinator.cc
@@ -14,6 +14,7 @@
#include "chrome/browser/ui/views/side_panel/read_anything/read_anything_toolbar_view.h"
#include "chrome/browser/ui/views/side_panel/side_panel_entry.h"
#include "chrome/browser/ui/views/side_panel/side_panel_registry.h"
+#include "chrome/browser/ui/webui/side_panel/read_anything/read_anything_prefs.h"
#include "chrome/browser/ui/webui/side_panel/read_anything/read_anything_ui.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/grit/generated_resources.h"
@@ -22,7 +23,12 @@
ReadAnythingCoordinator::ReadAnythingCoordinator(Browser* browser)
: BrowserUserData<ReadAnythingCoordinator>(*browser) {
// Create the model.
- model_ = std::make_unique<ReadAnythingModel>();
+ std::string prefs_font_name;
+ if (browser->profile() && browser->profile()->GetPrefs()) {
+ prefs_font_name = browser->profile()->GetPrefs()->GetString(
+ prefs::kAccessibilityReadAnythingFontName);
+ }
+ model_ = std::make_unique<ReadAnythingModel>(prefs_font_name);
// Create the controller.
controller_ = std::make_unique<ReadAnythingController>(model_.get(), browser);
diff --git a/chrome/browser/ui/views/side_panel/read_anything/read_anything_coordinator_unittest.cc b/chrome/browser/ui/views/side_panel/read_anything/read_anything_coordinator_unittest.cc
index 4982b24..c0cb260 100644
--- a/chrome/browser/ui/views/side_panel/read_anything/read_anything_coordinator_unittest.cc
+++ b/chrome/browser/ui/views/side_panel/read_anything/read_anything_coordinator_unittest.cc
@@ -30,14 +30,14 @@
class MockReadAnythingModelObserver : public ReadAnythingModel::Observer {
public:
MOCK_METHOD(void,
+ OnFontNameUpdated,
+ (const std::string& new_font_name),
+ (override));
+ MOCK_METHOD(void,
OnAXTreeDistilled,
(const ui::AXTreeUpdate& snapshot,
const std::vector<ui::AXNodeID>& content_node_ids),
(override));
- MOCK_METHOD(void,
- OnFontNameUpdated,
- (const std::string& new_font_name),
- (override));
};
class ReadAnythingCoordinatorTest : public TestWithBrowserView {
@@ -119,7 +119,7 @@
GetModel()->SetDistilledAXTree(ui::AXTreeUpdate(),
std::vector<ui::AXNodeID>());
- GetModel()->SetSelectedFontIndex(3);
+ GetModel()->SetSelectedFontByIndex(3);
GetModel()->RemoveObserver(&model_observer_);
}
diff --git a/chrome/browser/ui/views/side_panel/read_anything/read_anything_model.cc b/chrome/browser/ui/views/side_panel/read_anything/read_anything_model.cc
index 141b049..11bfdb1 100644
--- a/chrome/browser/ui/views/side_panel/read_anything/read_anything_model.cc
+++ b/chrome/browser/ui/views/side_panel/read_anything/read_anything_model.cc
@@ -8,28 +8,44 @@
#include <utility>
#include "base/check.h"
+#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
-ReadAnythingModel::ReadAnythingModel()
- : font_model_(std::make_unique<ReadAnythingFontModel>()) {}
+ReadAnythingModel::ReadAnythingModel(std::string prefs_font_name)
+ : font_model_(std::make_unique<ReadAnythingFontModel>()) {
+ // If this profile has previously selected a preferred font name choice,
+ // check that it is still a valid font, and if so, make it the default.
+ if (font_model_->IsValidFontName(prefs_font_name)) {
+ font_model_->SetDefaultIndexFromPrefsFontName(prefs_font_name);
+ font_name_ = prefs_font_name;
+ }
+}
+
ReadAnythingModel::~ReadAnythingModel() = default;
void ReadAnythingModel::AddObserver(Observer* obs) {
observers_.AddObserver(obs);
+ NotifyFontNameUpdated();
+ NotifyAXTreeDistilled();
}
void ReadAnythingModel::RemoveObserver(Observer* obs) {
observers_.RemoveObserver(obs);
}
-void ReadAnythingModel::SetSelectedFontIndex(int new_index) {
- font_name_ = font_model_->GetCurrentFontName(new_index);
+void ReadAnythingModel::SetSelectedFontByIndex(int new_index) {
+ // Check that the index is valid.
+ DCHECK(font_model_->IsValidFontIndex(new_index));
+
+ // Update state and notify listeners
+ font_name_ = font_model_->GetFontNameAt(new_index);
NotifyFontNameUpdated();
}
void ReadAnythingModel::SetDistilledAXTree(
ui::AXTreeUpdate snapshot,
std::vector<ui::AXNodeID> content_node_ids) {
+ // Update state and notify listeners
snapshot_ = std::move(snapshot);
content_node_ids_ = std::move(content_node_ids);
NotifyAXTreeDistilled();
@@ -58,9 +74,24 @@
font_choices_.shrink_to_fit();
}
+bool ReadAnythingFontModel::IsValidFontName(const std::string& font_name) {
+ return std::find(font_choices_.begin(), font_choices_.end(),
+ base::UTF8ToUTF16(font_name)) != font_choices_.end();
+}
+
+bool ReadAnythingFontModel::IsValidFontIndex(int index) {
+ return index >= 0 && index <= GetItemCount();
+}
+
+void ReadAnythingFontModel::SetDefaultIndexFromPrefsFontName(
+ std::string prefs_font_name) {
+ auto it = std::find(font_choices_.begin(), font_choices_.end(),
+ base::UTF8ToUTF16(prefs_font_name));
+ default_index_ = it - font_choices_.begin();
+}
+
int ReadAnythingFontModel::GetDefaultIndex() const {
- // TODO(1266555): This should be set on initialization based on Prefs.
- return 0;
+ return default_index_;
}
int ReadAnythingFontModel::GetItemCount() const {
@@ -77,7 +108,7 @@
return font_choices_.at(index);
}
-std::string ReadAnythingFontModel::GetCurrentFontName(int index) {
+std::string ReadAnythingFontModel::GetFontNameAt(int index) {
DCHECK(index >= 0 && index < GetItemCount());
return base::UTF16ToUTF8(font_choices_.at(index));
}
diff --git a/chrome/browser/ui/views/side_panel/read_anything/read_anything_model.h b/chrome/browser/ui/views/side_panel/read_anything/read_anything_model.h
index 4fd08f04..7e2e1f5e 100644
--- a/chrome/browser/ui/views/side_panel/read_anything/read_anything_model.h
+++ b/chrome/browser/ui/views/side_panel/read_anything/read_anything_model.h
@@ -30,7 +30,10 @@
ReadAnythingFontModel& operator=(const ReadAnythingFontModel&) = delete;
~ReadAnythingFontModel() override;
- std::string GetCurrentFontName(int index);
+ std::string GetFontNameAt(int index);
+ bool IsValidFontName(const std::string& font_name);
+ bool IsValidFontIndex(int index);
+ void SetDefaultIndexFromPrefsFontName(std::string prefs_font_name);
protected:
// ui::Combobox implementation:
@@ -42,6 +45,9 @@
private:
// Styled font names for the drop down options in front-end.
std::vector<std::u16string> font_choices_;
+
+ // Default index for drop down, either zero or populated from prefs.
+ int default_index_ = 0;
};
///////////////////////////////////////////////////////////////////////////////
@@ -55,13 +61,13 @@
public:
class Observer : public base::CheckedObserver {
public:
+ virtual void OnFontNameUpdated(const std::string& new_font_name) = 0;
virtual void OnAXTreeDistilled(
const ui::AXTreeUpdate& snapshot,
const std::vector<ui::AXNodeID>& content_node_ids) = 0;
- virtual void OnFontNameUpdated(const std::string& new_font_name) = 0;
};
- ReadAnythingModel();
+ explicit ReadAnythingModel(std::string prefs_font_name);
ReadAnythingModel(const ReadAnythingModel&) = delete;
ReadAnythingModel& operator=(const ReadAnythingModel&) = delete;
~ReadAnythingModel();
@@ -71,7 +77,7 @@
void SetDistilledAXTree(ui::AXTreeUpdate snapshot,
std::vector<ui::AXNodeID> content_node_ids);
- void SetSelectedFontIndex(int new_index);
+ void SetSelectedFontByIndex(int new_index);
ReadAnythingFontModel* GetFontModel() { return font_model_.get(); }
diff --git a/chrome/browser/ui/webui/side_panel/read_anything/read_anything_page_handler.cc b/chrome/browser/ui/webui/side_panel/read_anything/read_anything_page_handler.cc
index 4582be7..9e644c7 100644
--- a/chrome/browser/ui/webui/side_panel/read_anything/read_anything_page_handler.cc
+++ b/chrome/browser/ui/webui/side_panel/read_anything/read_anything_page_handler.cc
@@ -25,9 +25,11 @@
return;
coordinator_ = ReadAnythingCoordinator::FromBrowser(browser_);
- coordinator_->AddObserver(this);
+ if (coordinator_)
+ coordinator_->AddObserver(this);
model_ = coordinator_->GetModel();
- model_->AddObserver(this);
+ if (model_)
+ model_->AddObserver(this);
delegate_ = static_cast<ReadAnythingPageHandler::Delegate*>(
coordinator_->GetController());
if (delegate_)
@@ -59,5 +61,5 @@
void ReadAnythingPageHandler::OnFontNameUpdated(
const std::string& new_font_name) {
- page_->OnFontNameChange(new_font_name);
+ page_->OnFontNameChange(std::move(new_font_name));
}
diff --git a/chrome/browser/ui/webui/side_panel/read_anything/read_anything_page_handler.h b/chrome/browser/ui/webui/side_panel/read_anything/read_anything_page_handler.h
index 8b8c4be0..699ea390 100644
--- a/chrome/browser/ui/webui/side_panel/read_anything/read_anything_page_handler.h
+++ b/chrome/browser/ui/webui/side_panel/read_anything/read_anything_page_handler.h
@@ -48,10 +48,10 @@
~ReadAnythingPageHandler() override;
// ReadAnythingModel::Observer:
+ void OnFontNameUpdated(const std::string& new_font_name) override;
void OnAXTreeDistilled(
const ui::AXTreeUpdate& snapshot,
const std::vector<ui::AXNodeID>& content_node_ids) override;
- void OnFontNameUpdated(const std::string& new_font_name) override;
// ReadAnythingCoordinator::Observer:
void OnCoordinatorDestroyed() override;
diff --git a/chrome/browser/ui/webui/side_panel/read_anything/read_anything_prefs.cc b/chrome/browser/ui/webui/side_panel/read_anything/read_anything_prefs.cc
new file mode 100644
index 0000000..3a3dbd9f
--- /dev/null
+++ b/chrome/browser/ui/webui/side_panel/read_anything/read_anything_prefs.cc
@@ -0,0 +1,26 @@
+// Copyright 2022 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/webui/side_panel/read_anything/read_anything_prefs.h"
+
+#include "chrome/browser/ui/views/side_panel/read_anything/read_anything_constants.h"
+#include "components/pref_registry/pref_registry_syncable.h"
+
+namespace prefs {
+
+#if !BUILDFLAG(IS_ANDROID)
+// String to represent the user's preferred font for the read anything UI.
+const char kAccessibilityReadAnythingFontName[] =
+ "settings.a11y.read_anything.font_name";
+
+} // namespace prefs
+
+void RegisterReadAnythingProfilePrefs(
+ user_prefs::PrefRegistrySyncable* registry) {
+ registry->RegisterStringPref(prefs::kAccessibilityReadAnythingFontName,
+ kReadAnythingDefaultFontName,
+ user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
+}
+
+#endif // !BUILDFLAG(IS_ANDROID)
diff --git a/chrome/browser/ui/webui/side_panel/read_anything/read_anything_prefs.h b/chrome/browser/ui/webui/side_panel/read_anything/read_anything_prefs.h
new file mode 100644
index 0000000..b55604a6
--- /dev/null
+++ b/chrome/browser/ui/webui/side_panel/read_anything/read_anything_prefs.h
@@ -0,0 +1,28 @@
+// Copyright 2022 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.
+
+#ifndef CHROME_BROWSER_UI_WEBUI_SIDE_PANEL_READ_ANYTHING_READ_ANYTHING_PREFS_H_
+#define CHROME_BROWSER_UI_WEBUI_SIDE_PANEL_READ_ANYTHING_READ_ANYTHING_PREFS_H_
+
+#include "build/build_config.h"
+#include "chrome/common/buildflags.h"
+
+namespace user_prefs {
+class PrefRegistrySyncable;
+}
+
+namespace prefs {
+
+#if !BUILDFLAG(IS_ANDROID)
+
+extern const char kAccessibilityReadAnythingFontName[];
+
+} // namespace prefs
+
+void RegisterReadAnythingProfilePrefs(
+ user_prefs::PrefRegistrySyncable* registry);
+
+#endif // !BUILDFLAG(IS_ANDROID)
+
+#endif // CHROME_BROWSER_UI_WEBUI_SIDE_PANEL_READ_ANYTHING_READ_ANYTHING_PREFS_H_
