Bug 1561135 - Don't automatically enable font inflation when automatic font size adjustment is enabled in GV. r=snorp
☠☠ backed out by b318af7a75a3 ☠ ☠
authorDylan Roeh <droeh@mozilla.com>
Thu, 27 Jun 2019 21:50:03 +0000
changeset 543277 3ec2481cd32dc0e6a9a49ad6ba206edc1251592a
parent 543276 aaa160599ef488a6854c34a566b92abf5c2353c4
child 543278 9827057d4a865bc357ebb4fd72c7fc423b644130
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1561135
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1561135 - Don't automatically enable font inflation when automatic font size adjustment is enabled in GV. r=snorp Differential Revision: https://phabricator.services.mozilla.com/D35819
mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/RuntimeSettingsTest.kt
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoFontScaleListener.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntimeSettings.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
@@ -932,11 +932,12 @@ public class GeckoApplication extends Ap
         }
     }
 
     @Override // OnSharedPreferenceChangeListener
     public void onSharedPreferenceChanged(SharedPreferences prefs, String key) {
         if (GeckoPreferences.PREFS_SYSTEM_FONT_SIZE.equals(key)) {
             final boolean enabled = prefs.getBoolean(GeckoPreferences.PREFS_SYSTEM_FONT_SIZE, false);
             getRuntime().getSettings().setAutomaticFontSizeAdjustment(enabled);
+            getRuntime().getSettings().setFontInflationEnabled(enabled);
         }
     }
 }
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/RuntimeSettingsTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/RuntimeSettingsTest.kt
@@ -36,17 +36,17 @@ class RuntimeSettingsTest : BaseSessionT
 
         settings.automaticFontSizeAdjustment = true
         val contentResolver = InstrumentationRegistry.getTargetContext().contentResolver
         val expectedFontSizeFactor = Settings.System.getFloat(contentResolver,
                 Settings.System.FONT_SCALE, 1.0f)
         assertThat("Gecko font scale should match system font scale",
                 settings.fontSizeFactor.toDouble(), closeTo(expectedFontSizeFactor.toDouble(), 0.05))
         assertThat("font inflation enabled",
-                settings.fontInflationEnabled, `is`(true))
+                settings.fontInflationEnabled, `is`(initialFontInflation))
 
         settings.automaticFontSizeAdjustment = false
         assertThat("Gecko font scale restored to previous value",
                 settings.fontSizeFactor.toDouble(), closeTo(initialFontSize.toDouble(), 0.05))
         assertThat("font inflation restored to previous value",
                 settings.fontInflationEnabled, `is`(initialFontInflation))
 
         // Now check with that with font inflation initially off, the initial state is still
@@ -60,17 +60,17 @@ class RuntimeSettingsTest : BaseSessionT
         settings.fontInflationEnabled = initialFontInflation
         assertThat("font inflation initially set to $initialFontInflation",
                 settings.fontInflationEnabled, `is`(initialFontInflation))
 
         settings.automaticFontSizeAdjustment = true
         assertThat("Gecko font scale should match system font scale",
                 settings.fontSizeFactor.toDouble(), closeTo(expectedFontSizeFactor.toDouble(), 0.05))
         assertThat("font inflation enabled",
-                settings.fontInflationEnabled, `is`(true))
+                settings.fontInflationEnabled, `is`(initialFontInflation))
 
         settings.automaticFontSizeAdjustment = false
         assertThat("Gecko font scale restored to previous value",
                 settings.fontSizeFactor.toDouble(), closeTo(initialFontSize.toDouble(), 0.05))
         assertThat("font inflation restored to previous value",
                 settings.fontInflationEnabled, `is`(initialFontInflation))
     }
 
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoFontScaleListener.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoFontScaleListener.java
@@ -13,17 +13,17 @@ import android.content.ContentResolver;
 import android.content.Context;
 import android.database.ContentObserver;
 import android.net.Uri;
 import android.provider.Settings;
 import android.support.annotation.UiThread;
 import android.util.Log;
 
 /**
- * A class that automatically adjusts font size and font inflation settings for web content in Gecko
+ * A class that automatically adjusts font size settings for web content in Gecko
  * in accordance with the device's OS font scale setting.
  *
  * @see android.provider.Settings.System#FONT_SCALE
  */
 /* package */ final class GeckoFontScaleListener
         extends ContentObserver {
     private static final String LOGTAG = "GeckoFontScaleListener";
 
@@ -36,17 +36,16 @@ import android.util.Log;
     private Context mApplicationContext;
     private GeckoRuntimeSettings mSettings;
 
     private boolean mAttached;
     private boolean mEnabled;
     private boolean mRunning;
 
     private float mPrevGeckoFontScale;
-    private boolean mPrevFontInflationState;
 
     public static GeckoFontScaleListener getInstance() {
         return sInstance;
     }
 
     private GeckoFontScaleListener() {
         // Ensure the ContentObserver callback runs on the UI thread.
         super(ThreadUtils.getUiHandler());
@@ -126,17 +125,16 @@ import android.util.Log;
     }
 
     private void start() {
         if (mRunning) {
             return;
         }
 
         mPrevGeckoFontScale = mSettings.getFontSizeFactor();
-        mPrevFontInflationState = mSettings.getFontInflationEnabled();
         ContentResolver contentResolver = mApplicationContext.getContentResolver();
         Uri fontSizeSetting = Settings.System.getUriFor(Settings.System.FONT_SCALE);
         contentResolver.registerContentObserver(fontSizeSetting, false, this);
         onSystemFontScaleChange(contentResolver, false);
 
         mRunning = true;
     }
 
@@ -150,27 +148,23 @@ import android.util.Log;
         onSystemFontScaleChange(contentResolver, /*stopping*/ true);
 
         mRunning = false;
     }
 
     private void onSystemFontScaleChange(final ContentResolver contentResolver,
                                          final boolean stopping) {
         float fontScale;
-        boolean fontInflationEnabled;
 
         if (!stopping) { // Either we were enabled, or else the system font scale changed.
             fontScale = Settings.System.getFloat(contentResolver, Settings.System.FONT_SCALE, DEFAULT_FONT_SCALE);
-            fontInflationEnabled = true;
         } else { // We were turned off.
             fontScale = mPrevGeckoFontScale;
-            fontInflationEnabled = mPrevFontInflationState;
         }
 
-        mSettings.setFontInflationEnabledInternal(fontInflationEnabled);
         mSettings.setFontSizeFactorInternal(fontScale);
     }
 
     @UiThread // See constructor.
     @Override
     public void onChange(final boolean selfChange) {
         onSystemFontScaleChange(mApplicationContext.getContentResolver(), false);
     }
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntimeSettings.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntimeSettings.java
@@ -738,20 +738,20 @@ public final class GeckoRuntimeSettings 
      * @return True if console output is enabled.
      */
     public boolean getConsoleOutputEnabled() {
         return mConsoleOutput.get();
     }
 
     /**
      * Set whether or not font sizes in web content should be automatically scaled according to
-     * the device's current system font scale setting.
-     * Disabling this setting will restore the previously used values for
-     * {@link GeckoRuntimeSettings#getFontSizeFactor()} and
-     * {@link GeckoRuntimeSettings#getFontInflationEnabled()}.
+     * the device's current system font scale setting. Enabling this will prevent modification of
+     * {@link GeckoRuntimeSettings#setFontSizeFactor()}.
+     * Disabling this setting will restore the previously used value for
+     * {@link GeckoRuntimeSettings#getFontSizeFactor()}.
      *
      * @param enabled A flag determining whether or not font sizes should be scaled automatically
      *                to match the device's system font scale.
      * @return This GeckoRuntimeSettings instance.
      */
     public @NonNull GeckoRuntimeSettings setAutomaticFontSizeAdjustment(final boolean enabled) {
         GeckoFontScaleListener.getInstance().setEnabled(enabled);
         return this;
@@ -830,17 +830,17 @@ public final class GeckoRuntimeSettings 
 
     /* package */ @NonNull GeckoRuntimeSettings setFontSizeFactorInternal(
             final float fontSizeFactor) {
         if (fontSizeFactor < 0) {
             throw new IllegalArgumentException("fontSizeFactor cannot be < 0");
         }
 
         final int fontSizePercentage = Math.round(fontSizeFactor * 100);
-        mFontSizeFactor.commit(Math.round(fontSizePercentage));
+        mFontSizeFactor.commit(fontSizePercentage);
         if (getFontInflationEnabled()) {
             final int scaledFontInflation = Math.round(FONT_INFLATION_BASE_VALUE * fontSizeFactor);
             mFontInflationMinTwips.commit(scaledFontInflation);
         }
         return this;
     }
 
     /**
@@ -861,31 +861,20 @@ public final class GeckoRuntimeSettings 
      * {@link GeckoSessionSettings#VIEWPORT_MODE_MOBILE}. To improve readability, the font inflation
      * logic will attempt to increase font sizes for the main text content of the page only.
      *
      * <p>The magnitude of font inflation applied depends on the
      * {@link GeckoRuntimeSettings#setFontSizeFactor font size factor} currently in use.
      *
      * <p>Currently, any changes only take effect after a reload of the session.
      *
-     * <p>This setting cannot be modified while
-     * {@link GeckoRuntimeSettings#setAutomaticFontSizeAdjustment automatic font size adjustment}
-     * is enabled.
-     *
      * @param enabled A flag determining whether or not font inflation should be enabled.
      * @return This GeckoRuntimeSettings instance.
      */
     public @NonNull GeckoRuntimeSettings setFontInflationEnabled(final boolean enabled) {
-        if (getAutomaticFontSizeAdjustment()) {
-            throw new IllegalStateException("Not allowed when automatic font size adjustment is enabled");
-        }
-        return setFontInflationEnabledInternal(enabled);
-    }
-
-    /* package */ @NonNull GeckoRuntimeSettings setFontInflationEnabledInternal(final boolean enabled) {
         final int minTwips =
                 enabled ? Math.round(FONT_INFLATION_BASE_VALUE * getFontSizeFactor()) : 0;
         mFontInflationMinTwips.commit(minTwips);
         return this;
     }
 
     /**
      * Get whether or not font inflation for non mobile-friendly pages is currently enabled.
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
@@ -3,16 +3,23 @@ layout: default
 title: API Changelog
 description: GeckoView API Changelog.
 nav_exclude: true
 exclude: true
 ---
 
 <h1> GeckoView API Changelog. </h1>
 
+## v69
+- Modified behavior of ['setAutomaticFontSizeAdjustment'][69.1] so that it no 
+  longer has any effect on ['setFontInflationEnabled'][69.2]
+
+[69.1]: ./GeckoRuntimeSettings.html#setAutomaticFontSizeAdjustment-boolean-
+[69.2]: ./GeckoRuntimeSettings.html#setFontInflationEnabled-boolean-
+
 ## v68
 - Added [`GeckoRuntime#configurationChanged`][68.1] to notify the device
   configuration has changed.
 
 [68.1]: ../GeckoRuntime.html#configurationChanged
 
 - Added `onSessionStateChange` to [`ProgressDelegate`][68.2] and removed `saveState`.