0

Move force enable zoom preference out of FontSizePrefs

This CL moves the management of the force enable zoom user preference
out of the FontSizePrefs singleton and into the AccessibilitySettings
delegate. The user preference is no longer tied to the text scaling
setting, which has been removed, so it is misplaced being in the
FontSizePrefs class, which we hope to eventually remove entirely. There
is no change to the user preference or setting, merely moving where the
string is defined and how the AccessibilitySettings page checks and
updates the value.

AX-Relnotes: N/A
Bug: 376543349
Change-Id: If4821349b5443d40b12528f1f8f468111fc3a68b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6021506
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Cr-Commit-Position: refs/heads/main@{#1383257}
This commit is contained in:
Mark Schillaci
2024-11-14 21:53:21 +00:00
committed by Chromium LUCI CQ
parent 67d8019122
commit 88644df830
13 changed files with 71 additions and 126 deletions
chrome
components/browser_ui/accessibility/android

@@ -51,7 +51,6 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/ViewportTestUtils.java",
"javatests/src/org/chromium/chrome/browser/VirtualKeyboardResizeTest.java",
"javatests/src/org/chromium/chrome/browser/WarmupManagerTest.java",
"javatests/src/org/chromium/chrome/browser/accessibility/FontSizePrefsTest.java",
"javatests/src/org/chromium/chrome/browser/accessibility/settings/AccessibilitySettingsTest.java",
"javatests/src/org/chromium/chrome/browser/app/ContextMenuDragTest.java",
"javatests/src/org/chromium/chrome/browser/app/appmenu/OverviewAppMenuTest.java",

@@ -8,7 +8,6 @@ import android.content.Intent;
import android.os.Bundle;
import android.provider.Settings;
import androidx.annotation.VisibleForTesting;
import androidx.preference.Preference;
import androidx.preference.PreferenceFragmentCompat;
@@ -20,7 +19,6 @@ import org.chromium.chrome.browser.image_descriptions.ImageDescriptionsControlle
import org.chromium.chrome.browser.preferences.Pref;
import org.chromium.chrome.browser.settings.SettingsNavigationFactory;
import org.chromium.components.browser_ui.accessibility.AccessibilitySettingsDelegate;
import org.chromium.components.browser_ui.accessibility.FontSizePrefs;
import org.chromium.components.browser_ui.accessibility.PageZoomPreference;
import org.chromium.components.browser_ui.accessibility.PageZoomUma;
import org.chromium.components.browser_ui.accessibility.PageZoomUtils;
@@ -58,9 +56,6 @@ public class AccessibilitySettings extends PreferenceFragmentCompat
private double mPageZoomLatestDefaultZoomPrefValue;
private PrefService mPrefService;
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
public FontSizePrefs mFontSizePrefs;
private final ObservableSupplierImpl<String> mPageTitle = new ObservableSupplierImpl<>();
public void setPrefService(PrefService prefService) {
@@ -69,7 +64,6 @@ public class AccessibilitySettings extends PreferenceFragmentCompat
public void setDelegate(AccessibilitySettingsDelegate delegate) {
mDelegate = delegate;
mFontSizePrefs = FontSizePrefs.getInstance(delegate.getBrowserContextHandle());
}
@Override
@@ -109,7 +103,8 @@ public class AccessibilitySettings extends PreferenceFragmentCompat
mForceEnableZoomPref = (ChromeSwitchPreference) findPreference(PREF_FORCE_ENABLE_ZOOM);
mForceEnableZoomPref.setOnPreferenceChangeListener(this);
mForceEnableZoomPref.setChecked(mFontSizePrefs.getForceEnableZoom());
mForceEnableZoomPref.setChecked(
mDelegate.getForceEnableZoomAccessibilityDelegate().getValue());
mJumpStartOmnibox =
(ChromeSwitchPreference) findPreference(OmniboxFeatures.KEY_JUMP_START_OMNIBOX);
@@ -183,7 +178,7 @@ public class AccessibilitySettings extends PreferenceFragmentCompat
@Override
public boolean onPreferenceChange(Preference preference, Object newValue) {
if (PREF_FORCE_ENABLE_ZOOM.equals(preference.getKey())) {
mFontSizePrefs.setForceEnableZoom((Boolean) newValue);
mDelegate.getForceEnableZoomAccessibilityDelegate().setValue((Boolean) newValue);
} else if (PREF_READER_FOR_ACCESSIBILITY.equals(preference.getKey())) {
mPrefService.setBoolean(Pref.READER_FOR_ACCESSIBILITY, (Boolean) newValue);
} else if (PREF_PAGE_ZOOM_DEFAULT_ZOOM.equals(preference.getKey())) {

@@ -33,10 +33,31 @@ public class ChromeAccessibilitySettingsDelegate implements AccessibilitySetting
}
}
private static class ForceEnableZoomAccessibilityDelegate implements BooleanPreferenceDelegate {
private final BrowserContextHandle mBrowserContextHandle;
public ForceEnableZoomAccessibilityDelegate(BrowserContextHandle mBrowserContextHandle) {
this.mBrowserContextHandle = mBrowserContextHandle;
}
@Override
public boolean getValue() {
return UserPrefs.get(mBrowserContextHandle)
.getBoolean(Pref.ACCESSIBILITY_FORCE_ENABLE_ZOOM);
}
@Override
public void setValue(boolean value) {
UserPrefs.get(mBrowserContextHandle)
.setBoolean(Pref.ACCESSIBILITY_FORCE_ENABLE_ZOOM, value);
}
}
private final Profile mProfile;
/**
* Constructs a delegate for the given profile.
*
* @param profile The profile associated with the delegate.
*/
public ChromeAccessibilitySettingsDelegate(Profile profile) {
@@ -52,4 +73,9 @@ public class ChromeAccessibilitySettingsDelegate implements AccessibilitySetting
public IntegerPreferenceDelegate getTextSizeContrastAccessibilityDelegate() {
return new TextSizeContrastAccessibilityDelegate(getBrowserContextHandle());
}
@Override
public BooleanPreferenceDelegate getForceEnableZoomAccessibilityDelegate() {
return new ForceEnableZoomAccessibilityDelegate(getBrowserContextHandle());
}
}

@@ -1,64 +0,0 @@
// Copyright 2014 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.accessibility;
import android.content.Context;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.filters.SmallTest;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.Feature;
import org.chromium.chrome.browser.profiles.ProfileManager;
import org.chromium.chrome.test.ChromeBrowserTestRule;
import org.chromium.components.browser_ui.accessibility.FontSizePrefs;
/**
* Tests for {@link FontSizePrefs}.
*
* <p>TODO(crbug.com/40214849): This tests the class in //components/browser_ui, but we don't have a
* good way of testing with native code there.
*/
@RunWith(BaseJUnit4ClassRunner.class)
public class FontSizePrefsTest {
@Rule public final ChromeBrowserTestRule mBrowserTestRule = new ChromeBrowserTestRule();
private FontSizePrefs mFontSizePrefs;
@Before
public void setUp() {
Context context = ApplicationProvider.getApplicationContext();
mFontSizePrefs = getFontSizePrefs(context);
}
@Test
@SmallTest
@Feature({"Accessibility"})
public void testForceEnableZoom() {
Assert.assertFalse(getForceEnableZoom());
setForceEnableZoom(true);
Assert.assertTrue(getForceEnableZoom());
}
private FontSizePrefs getFontSizePrefs(final Context context) {
return ThreadUtils.runOnUiThreadBlocking(
() -> FontSizePrefs.getInstance(ProfileManager.getLastUsedRegularProfile()));
}
private void setForceEnableZoom(final boolean enabled) {
ThreadUtils.runOnUiThreadBlocking(() -> mFontSizePrefs.setForceEnableZoom(enabled));
}
private boolean getForceEnableZoom() {
return ThreadUtils.runOnUiThreadBlocking(() -> mFontSizePrefs.getForceEnableZoom());
}
}

@@ -41,12 +41,15 @@ import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Features;
import org.chromium.base.test.util.Features.DisableFeatures;
import org.chromium.base.test.util.Features.EnableFeatures;
import org.chromium.chrome.browser.preferences.Pref;
import org.chromium.chrome.browser.profiles.ProfileManager;
import org.chromium.chrome.browser.settings.SettingsActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.R;
import org.chromium.components.browser_ui.accessibility.PageZoomPreference;
import org.chromium.components.browser_ui.accessibility.PageZoomUtils;
import org.chromium.components.browser_ui.settings.ChromeSwitchPreference;
import org.chromium.components.user_prefs.UserPrefs;
import org.chromium.content_public.browser.ContentFeatureList;
import org.chromium.ui.accessibility.AccessibilityState;
import org.chromium.ui.test.util.ViewUtils;
@@ -114,7 +117,8 @@ public class AccessibilitySettingsTest {
() ->
Assert.assertTrue(
"Force enable zoom user pref was not updated on toggle",
mAccessibilitySettings.mFontSizePrefs.getForceEnableZoom()));
UserPrefs.get(ProfileManager.getLastUsedRegularProfile())
.getBoolean(Pref.ACCESSIBILITY_FORCE_ENABLE_ZOOM)));
}
@Test

@@ -4462,7 +4462,7 @@ void ChromeContentBrowserClient::OverrideWebkitPrefs(
web_prefs->text_size_contrast_factor =
prefs->GetInteger(prefs::kAccessibilityTextSizeContrastFactor);
web_prefs->force_enable_zoom =
prefs->GetBoolean(browser_ui::prefs::kWebKitForceEnableZoom);
prefs->GetBoolean(prefs::kAccessibilityForceEnableZoom);
web_prefs->font_weight_adjustment =
prefs->GetInteger(prefs::kAccessibilityFontWeightAdjustment);
#endif

@@ -56,7 +56,7 @@ const char* const kWebPrefsToObserve[] = {
#if BUILDFLAG(IS_ANDROID)
browser_ui::prefs::kWebKitFontScaleFactor,
prefs::kAccessibilityTextSizeContrastFactor,
browser_ui::prefs::kWebKitForceEnableZoom,
prefs::kAccessibilityForceEnableZoom,
prefs::kAccessibilityFontWeightAdjustment,
prefs::kWebKitPasswordEchoEnabled,
#endif

@@ -380,7 +380,7 @@ void PrefsTabHelper::RegisterProfilePrefs(
#if BUILDFLAG(IS_ANDROID)
registry->RegisterDoublePref(browser_ui::prefs::kWebKitFontScaleFactor, 1.0);
registry->RegisterIntegerPref(prefs::kAccessibilityTextSizeContrastFactor, 0);
registry->RegisterBooleanPref(browser_ui::prefs::kWebKitForceEnableZoom,
registry->RegisterBooleanPref(prefs::kAccessibilityForceEnableZoom,
pref_defaults.force_enable_zoom);
registry->RegisterBooleanPref(prefs::kWebKitPasswordEchoEnabled,
pref_defaults.password_echo_enabled);

@@ -1816,6 +1816,10 @@ inline constexpr char kFullscreenAllowed[] = "fullscreen.allowed";
inline constexpr char kAccessibilityFontWeightAdjustment[] =
"settings.a11y.font_weight_adjustment";
// The user requested that Chrome try to override sites that disable zoom.
inline constexpr char kAccessibilityForceEnableZoom[] =
"webkit.webprefs.force_enable_zoom";
inline constexpr char kAccessibilityTextSizeContrastFactor[] =
"settings.a11y.text_size_contrast_factor";

@@ -22,7 +22,6 @@ using base::android::JavaRef;
namespace prefs {
const char kWebKitFontScaleFactor[] = "webkit.webprefs.font_scale_factor";
const char kWebKitForceEnableZoom[] = "webkit.webprefs.force_enable_zoom";
} // namespace prefs
FontSizePrefsAndroid::FontSizePrefsAndroid(
@@ -43,17 +42,6 @@ void FontSizePrefsAndroid::SetFontScaleFactor(JNIEnv* env,
static_cast<double>(font_scale_factor));
}
void FontSizePrefsAndroid::SetForceEnableZoom(JNIEnv* env,
const JavaRef<jobject>& obj,
jboolean enabled) {
pref_service_->SetBoolean(prefs::kWebKitForceEnableZoom, enabled);
}
bool FontSizePrefsAndroid::GetForceEnableZoom(JNIEnv* env,
const JavaRef<jobject>& obj) {
return pref_service_->GetBoolean(prefs::kWebKitForceEnableZoom);
}
void FontSizePrefsAndroid::Destroy(JNIEnv* env) {
delete this;
}

@@ -15,7 +15,6 @@ namespace prefs {
// Prefs related to this class.
extern const char kWebKitFontScaleFactor[];
extern const char kWebKitForceEnableZoom[];
} // namespace prefs
@@ -38,11 +37,6 @@ class FontSizePrefsAndroid {
void SetFontScaleFactor(JNIEnv* env,
const base::android::JavaRef<jobject>& obj,
jfloat font_scale_factor);
void SetForceEnableZoom(JNIEnv* env,
const base::android::JavaRef<jobject>& obj,
jboolean enabled);
bool GetForceEnableZoom(JNIEnv* env,
const base::android::JavaRef<jobject>& obj);
void Destroy(JNIEnv* env);
private:

@@ -22,12 +22,32 @@ public interface AccessibilitySettingsDelegate {
void setValue(int value);
}
/** @return The BrowserContextHandle that should be used to read and update settings. */
/** An interface to control a single integer preference. */
interface BooleanPreferenceDelegate {
/**
* @return boolean - Current value of the preference of this instance.
*/
boolean getValue();
/** Sets a new value for the preference of this instance. */
void setValue(boolean value);
}
/**
* @return The BrowserContextHandle that should be used to read and update settings.
*/
BrowserContextHandle getBrowserContextHandle();
/**
* @return the InterPreferenceDelegate instance that should be used for reading and setting the
* text size contrast value for accessibility settings. Return null to omit the preference.
* text size contrast value for accessibility settings. Return null to omit the preference.
*/
IntegerPreferenceDelegate getTextSizeContrastAccessibilityDelegate();
/**
* @return the BooleanPreferenceDelegate instance that should be used for reading and setting
* the force enable zoom value for accessibility settings. Return null to omit the
* preference.
*/
BooleanPreferenceDelegate getForceEnableZoomAccessibilityDelegate();
}

@@ -14,15 +14,14 @@ import org.chromium.base.ThreadUtils;
import org.chromium.content_public.browser.BrowserContextHandle;
/**
* Singleton class for user settings related to font sizes. This class manages the force enable zoom
* user setting, and the "font scale factor" setting. The font scale factor is equal to the OS-level
* Android "Settings > Accessibility > Display size and text > Font size" setting. This value was
* previously boosted using the user's Chrome text scaling setting, but that setting has been
* removed with the release of Accessibility Page Zoom (11/2024). The OS-level user setting is still
* read and passed to the Web Preference `font_scale_factor`, which is plumbed to the Blink code as
* the `accessibility_font_scale_factor`. This is used in the text_autosizer.cc file for sites that
* still use text-size-adjust. The value is no longer boosted, but future work may boost the value
* based on the user's zoom setting.
* Singleton class for user settings related to font sizes. This class manages the "font scale
* factor" setting. The font scale factor is equal to the OS-level Android "Settings > Accessibility
* > Display size and text > Font size" setting. This value was previously boosted using the user's
* Chrome text scaling setting, but that setting has been removed with the release of Accessibility
* Page Zoom (11/2024). The OS-level user setting is still read and passed to the Web Preference
* `font_scale_factor`, which is plumbed to the Blink code as the `accessibility_font_scale_factor`.
* This is used in the text_autosizer.cc file for sites that still use text-size-adjust. The value
* is no longer boosted, but future work may boost the value based on the user's zoom setting.
*/
@JNINamespace("browser_ui")
public class FontSizePrefs {
@@ -56,21 +55,6 @@ public class FontSizePrefs {
sFontSizePrefs = null;
}
/**
* Sets forceEnableZoom due to a user request (e.g. checking a checkbox). This implicitly sets
* userSetForceEnableZoom.
*/
public void setForceEnableZoom(boolean enabled) {
FontSizePrefsJni.get()
.setForceEnableZoom(mFontSizePrefsAndroidPtr, FontSizePrefs.this, enabled);
}
/** Returns whether forceEnableZoom is enabled. */
public boolean getForceEnableZoom() {
return FontSizePrefsJni.get()
.getForceEnableZoom(mFontSizePrefsAndroidPtr, FontSizePrefs.this);
}
/**
* Updates the fontScaleFactor based on the system-wide font scale.
*
@@ -96,10 +80,5 @@ public class FontSizePrefs {
void setFontScaleFactor(
long nativeFontSizePrefsAndroid, FontSizePrefs caller, float fontScaleFactor);
boolean getForceEnableZoom(long nativeFontSizePrefsAndroid, FontSizePrefs caller);
void setForceEnableZoom(
long nativeFontSizePrefsAndroid, FontSizePrefs caller, boolean enabled);
}
}