From 0ddf4f5090af2a3e2ee399e3f2694ea0ca23436b Mon Sep 17 00:00:00 2001
From: Scott <scott.downe@gmail.com>
Date: Tue, 3 Nov 2020 22:13:15 +0000
Subject: [PATCH] Bug 1674992 - Fix Pocket story region check showing flash of
 stories as it updates r=gvn

Differential Revision: https://phabricator.services.mozilla.com/D95765
---
 .../components/newtab/lib/ActivityStream.jsm  | 12 +++++++
 .../test/unit/lib/ActivityStream.test.js      | 33 +++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/browser/components/newtab/lib/ActivityStream.jsm b/browser/components/newtab/lib/ActivityStream.jsm
index 45ea05def2da..743e581f970b 100644
--- a/browser/components/newtab/lib/ActivityStream.jsm
+++ b/browser/components/newtab/lib/ActivityStream.jsm
@@ -512,6 +512,13 @@ const PREFS_CONFIG = new Map([
       getValue: ({ geo }) => {
         const preffedRegionsString =
           Services.prefs.getStringPref(REGION_BASIC_CONFIG) || "";
+        // If no regions are set to basic,
+        // we don't need to bother checking against the region.
+        // We are also not concerned if geo is not set,
+        // because stories are going to be empty until we have geo.
+        if (!preffedRegionsString) {
+          return false;
+        }
         const preffedRegions = preffedRegionsString
           .split(",")
           .map(s => s.trim());
@@ -592,6 +599,11 @@ const FEEDS_DATA = [
       "System pref that fetches content recommendations from a configurable content provider",
     // Dynamically determine if Pocket should be shown for a geo / locale
     getValue: ({ geo, locale }) => {
+      // If we don't have geo, we don't want to flash the screen with stories while geo loads.
+      // Best to display nothing until geo is ready.
+      if (!geo) {
+        return false;
+      }
       const preffedRegionsBlockString =
         Services.prefs.getStringPref(REGION_STORIES_BLOCK) || "";
       const preffedRegionsString =
diff --git a/browser/components/newtab/test/unit/lib/ActivityStream.test.js b/browser/components/newtab/test/unit/lib/ActivityStream.test.js
index fc7201a9b4d9..668afebd3759 100644
--- a/browser/components/newtab/test/unit/lib/ActivityStream.test.js
+++ b/browser/components/newtab/test/unit/lib/ActivityStream.test.js
@@ -207,6 +207,20 @@ describe("ActivityStream", () => {
         .stub(global.Services.locale, "appLocaleAsBCP47")
         .get(() => "en-CA");
     });
+    it("should enable 7 row layout pref if no basic config is set and no geo is set", () => {
+      getStringPrefStub
+        .withArgs(
+          "browser.newtabpage.activity-stream.discoverystream.region-basic-config"
+        )
+        .returns("");
+      sandbox.stub(global.Region, "home").get(() => "");
+
+      as._updateDynamicPrefs();
+
+      assert.isFalse(
+        PREFS_CONFIG.get("discoverystream.region-basic-layout").value
+      );
+    });
     it("should enable 1 row layout pref based on region layout pref", () => {
       getStringPrefStub
         .withArgs(
@@ -268,6 +282,25 @@ describe("ActivityStream", () => {
 
       assert.isFalse(PREFS_CONFIG.get("feeds.system.topstories").value);
     });
+    it("should be false with no geo but an allowed locale", () => {
+      appLocaleAsBCP47Stub.get(() => "");
+      sandbox.stub(global.Region, "home").get(() => "");
+      appLocaleAsBCP47Stub.get(() => "en-US");
+      getStringPrefStub
+        .withArgs(
+          "browser.newtabpage.activity-stream.discoverystream.locale-list-config"
+        )
+        .returns("en-US,en-CA,en-GB")
+        // We only have this pref set to trigger a close to real situation.
+        .withArgs(
+          "browser.newtabpage.activity-stream.discoverystream.region-stories-block"
+        )
+        .returns("FR");
+
+      as._updateDynamicPrefs();
+
+      assert.isFalse(PREFS_CONFIG.get("feeds.system.topstories").value);
+    });
     it("should be false with unexpected geo", () => {
       sandbox.stub(global.Region, "home").get(() => "NOGEO");