[WPE][GTK] Introduce NeedsUnbrandedUserAgent quirk and use it for acc…

…ounts.google.com, docs.google.com, and drive.google.com

https://bugs.webkit.org/show_bug.cgi?id=222978

Patch by Michael Catanzaro <mcatanzaro@gnome.org> on 2021-03-10
Reviewed by Carlos Garcia Campos.

Source/WebCore:

This is a follow-up to bug #222039. I simplified our Google user agent quirks too much in
that bug, breaking accounts.google.com, docs.google.com, and drive.google.com for clients
that set application name and version in the user agent. What we really need here is an
empty quirk in order to ensure our most boring standard user agent is used without any
application branding or customizations. But we no longer need to fake platform or browser,
as was required in the past.

Additionaly, clean up the code a bit. We shouldn't need to compute domain and baseDomain
many separate times, for instance. There's also no need to perform string operations to
add the WebKit version to the user agent, since the version has been frozen for several
years now and is likely to remain frozen indefinitely. Finally, remove some forgotten
leftovers of our Internet Explorer and Windows quirks that were previously used for Google
Docs.

* platform/UserAgentQuirks.cpp:
(WebCore::urlRequiresChromeBrowser):
(WebCore::urlRequiresFirefoxBrowser):
(WebCore::urlRequiresMacintoshPlatform):
(WebCore::urlRequiresUnbrandedUserAgent):
(WebCore::UserAgentQuirks::quirksForURL):
(WebCore::UserAgentQuirks::stringForQuirk):
(WebCore::isGoogle): Deleted.
(WebCore::urlRequiresLinuxDesktopPlatform): Deleted.
* platform/UserAgentQuirks.h:
* platform/glib/UserAgentGLib.cpp:
(WebCore::buildUserAgentString):
(WebCore::standardUserAgent):
(WebCore::standardUserAgentForURL):
(WebCore::versionForUAString): Deleted.

Tools:

* TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp:
(TestWebKitAPI::assertUserAgentForURLHasEmptyQuirk):
(TestWebKitAPI::TEST):
(TestWebKitAPI::assertUserAgentForURLHasLinuxPlatformQuirk): Deleted.

Canonical link: https://commits.webkit.org/235126@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@274210 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,40 @@
+2021-03-10  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [WPE][GTK] Introduce NeedsUnbrandedUserAgent quirk and use it for accounts.google.com, docs.google.com, and drive.google.com
+        https://bugs.webkit.org/show_bug.cgi?id=222978
+
+        Reviewed by Carlos Garcia Campos.
+
+        This is a follow-up to bug #222039. I simplified our Google user agent quirks too much in
+        that bug, breaking accounts.google.com, docs.google.com, and drive.google.com for clients
+        that set application name and version in the user agent. What we really need here is an
+        empty quirk in order to ensure our most boring standard user agent is used without any
+        application branding or customizations. But we no longer need to fake platform or browser,
+        as was required in the past.
+
+        Additionaly, clean up the code a bit. We shouldn't need to compute domain and baseDomain
+        many separate times, for instance. There's also no need to perform string operations to
+        add the WebKit version to the user agent, since the version has been frozen for several
+        years now and is likely to remain frozen indefinitely. Finally, remove some forgotten
+        leftovers of our Internet Explorer and Windows quirks that were previously used for Google
+        Docs.
+
+        * platform/UserAgentQuirks.cpp:
+        (WebCore::urlRequiresChromeBrowser):
+        (WebCore::urlRequiresFirefoxBrowser):
+        (WebCore::urlRequiresMacintoshPlatform):
+        (WebCore::urlRequiresUnbrandedUserAgent):
+        (WebCore::UserAgentQuirks::quirksForURL):
+        (WebCore::UserAgentQuirks::stringForQuirk):
+        (WebCore::isGoogle): Deleted.
+        (WebCore::urlRequiresLinuxDesktopPlatform): Deleted.
+        * platform/UserAgentQuirks.h:
+        * platform/glib/UserAgentGLib.cpp:
+        (WebCore::buildUserAgentString):
+        (WebCore::standardUserAgent):
+        (WebCore::standardUserAgentForURL):
+        (WebCore::versionForUAString): Deleted.
+
 2021-03-10  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, reverting r274166.

Source/WebCore/platform/UserAgentQuirks.cpp

@@ -34,38 +34,14 @@ namespace WebCore {
 
 // When editing the quirks in this file, be sure to update
 // Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp.
-
-static bool isGoogle(const URL& url)
-{
-    String domain = url.host().toString();
-    String baseDomain = topPrivatelyControlledDomain(domain);
-
-    // Our Google UA is *very* complicated to get right. Read
-    // https://webkit.org/b/142074 carefully before changing. Test that 3D
-    // view is available in Google Maps. Test Google Calendar. Test logging out
-    // and logging in to a Google account. Change platformVersionForUAString()
-    // to return "FreeBSD amd64" and test everything again.
-    if (baseDomain.startsWith("google."))
-        return true;
-    if (baseDomain == "gstatic.com")
-        return true;
-    if (baseDomain == "googleusercontent.com")
-        return true;
-    // googleapis.com is in the public suffix list, which is confusing. E.g.
-    // fonts.googleapis.com is actually a base domain.
-    if (domain.endsWith(".googleapis.com"))
-        return true;
-
-    return false;
-}
+//
+// When testing changes, be sure to test with application branding enabled.
+// Otherwise, we will not notice when urlRequiresUnbrandedUserAgent is needed.
 
 // Be careful with this quirk: it's an invitation for sites to use JavaScript
 // that works in Chrome that WebKit cannot handle. Prefer other quirks instead.
-static bool urlRequiresChromeBrowser(const URL& url)
+static bool urlRequiresChromeBrowser(const String& domain, const String& baseDomain)
 {
-    String domain = url.host().toString();
-    String baseDomain = topPrivatelyControlledDomain(domain);
-
     // Needed for fonts on many sites to work with WebKit.
     // https://bugs.webkit.org/show_bug.cgi?id=147296
     if (baseDomain == "typekit.net" || baseDomain == "typekit.com")
@@ -93,10 +69,8 @@ static bool urlRequiresChromeBrowser(const URL& url)
 // quirk is good for websites that do macOS-specific things we don't want on
 // other platforms, and when the risk of the website doing Firefox-specific
 // things is relatively low.
-static bool urlRequiresFirefoxBrowser(const URL& url)
+static bool urlRequiresFirefoxBrowser(const String& domain)
 {
-    String domain = url.host().toString();
-
     // Red Hat Bugzilla displays a warning page when performing searches with WebKitGTK's standard
     // user agent.
     if (domain == "bugzilla.redhat.com")
@@ -110,11 +84,8 @@ static bool urlRequiresFirefoxBrowser(const URL& url)
     return false;
 }
 
-static bool urlRequiresMacintoshPlatform(const URL& url)
+static bool urlRequiresMacintoshPlatform(const String& domain, const String& baseDomain)
 {
-    String domain = url.host().toString();
-    String baseDomain = topPrivatelyControlledDomain(domain);
-
     // At least finance.yahoo.com displays a mobile version with WebKitGTK's standard user agent.
     if (chassisType() != WTF::ChassisType::Mobile && baseDomain == "yahoo.com")
         return true;
@@ -147,26 +118,44 @@ static bool urlRequiresMacintoshPlatform(const URL& url)
     return false;
 }
 
-static bool urlRequiresLinuxDesktopPlatform(const URL& url)
+static bool urlRequiresUnbrandedUserAgent(const String& domain)
 {
-    return isGoogle(url) && chassisType() != WTF::ChassisType::Mobile;
+    // Google uses an ugly fallback login page if application branding is
+    // appended to WebKitGTK's standard user agent.
+    if (domain == "accounts.google.com")
+        return true;
+
+    // Google Docs displays an unsupported browser warning if application
+    // branding is appended to WebKitGTK's standard user agent.
+    if (domain == "docs.google.com")
+        return true;
+
+    // Google Drive displays an unsupported browser warning if application
+    // branding is appended to WebKitGTK's standard user agent.
+    if (domain == "drive.google.com")
+        return true;
+
+    return false;
 }
 
 UserAgentQuirks UserAgentQuirks::quirksForURL(const URL& url)
 {
     ASSERT(!url.isNull());
 
+    String domain = url.host().toString();
+    String baseDomain = topPrivatelyControlledDomain(domain);
     UserAgentQuirks quirks;
 
-    if (urlRequiresChromeBrowser(url))
+    if (urlRequiresChromeBrowser(domain, baseDomain))
         quirks.add(UserAgentQuirks::NeedsChromeBrowser);
-    else if (urlRequiresFirefoxBrowser(url))
+    else if (urlRequiresFirefoxBrowser(domain))
         quirks.add(UserAgentQuirks::NeedsFirefoxBrowser);
 
-    if (urlRequiresMacintoshPlatform(url))
+    if (urlRequiresMacintoshPlatform(domain, baseDomain))
         quirks.add(UserAgentQuirks::NeedsMacintoshPlatform);
-    else if (urlRequiresLinuxDesktopPlatform(url))
-        quirks.add(UserAgentQuirks::NeedsLinuxDesktopPlatform);
+
+    if (urlRequiresUnbrandedUserAgent(domain))
+        quirks.add(UserAgentQuirks::NeedsUnbrandedUserAgent);
 
     return quirks;
 }
@@ -181,10 +170,8 @@ String UserAgentQuirks::stringForQuirk(UserAgentQuirk quirk)
         return "; rv:87.0) Gecko/20100101 Firefox/87.0"_s;
     case NeedsMacintoshPlatform:
         return "Macintosh; Intel Mac OS X 10_15"_s;
-    case NeedsLinuxDesktopPlatform:
-        return "X11; Linux x86_64"_s;
+    case NeedsUnbrandedUserAgent:
     case NumUserAgentQuirks:
-    default:
         ASSERT_NOT_REACHED();
     }
     return ""_s;

Source/WebCore/platform/UserAgentQuirks.h

@@ -35,10 +35,8 @@ class UserAgentQuirks {
     enum UserAgentQuirk {
         NeedsChromeBrowser,
         NeedsFirefoxBrowser,
-        NeedsInternetExplorerBrowser,
         NeedsMacintoshPlatform,
-        NeedsWindowsPlatform,
-        NeedsLinuxDesktopPlatform,
+        NeedsUnbrandedUserAgent,
 
         NumUserAgentQuirks
     };

Source/WebCore/platform/glib/UserAgentGLib.cpp

@@ -75,12 +75,6 @@ static const String platformVersionForUAString()
 #endif
 }
 
-static inline const char* versionForUAString()
-{
-    // https://bugs.webkit.org/show_bug.cgi?id=180365
-    return "605.1.15";
-}
-
 static String buildUserAgentString(const UserAgentQuirks& quirks)
 {
     StringBuilder uaString;
@@ -89,8 +83,6 @@ static String buildUserAgentString(const UserAgentQuirks& quirks)
 
     if (quirks.contains(UserAgentQuirks::NeedsMacintoshPlatform))
         uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsMacintoshPlatform));
-    else if (quirks.contains(UserAgentQuirks::NeedsLinuxDesktopPlatform))
-        uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsLinuxDesktopPlatform));
     else {
         uaString.append(platformForUAString());
         uaString.appendLiteral("; ");
@@ -105,9 +97,7 @@ static String buildUserAgentString(const UserAgentQuirks& quirks)
         return uaString.toString();
     }
 
-    uaString.appendLiteral(") AppleWebKit/");
-    uaString.append(versionForUAString());
-    uaString.appendLiteral(" (KHTML, like Gecko) ");
+    uaString.appendLiteral(") AppleWebKit/605.1.15 (KHTML, like Gecko) ");
 
     // Note that Chrome UAs advertise *both* Chrome/X and Safari/X, but it does
     // not advertise Version/X.
@@ -121,8 +111,7 @@ static String buildUserAgentString(const UserAgentQuirks& quirks)
 
     if (chassisType() == WTF::ChassisType::Mobile)
         uaString.appendLiteral("Mobile ");
-    uaString.appendLiteral("Safari/");
-    uaString.append(versionForUAString());
+    uaString.appendLiteral("Safari/605.1.15");
 
     return uaString.toString();
 }
@@ -150,7 +139,7 @@ String standardUserAgent(const String& applicationName, const String& applicatio
     } else {
         String finalApplicationVersion = applicationVersion;
         if (finalApplicationVersion.isEmpty())
-            finalApplicationVersion = versionForUAString();
+            finalApplicationVersion = "605.1.15";
         userAgent = standardUserAgentStatic() + ' ' + applicationName + '/' + finalApplicationVersion;
     }
 
@@ -169,6 +158,8 @@ String standardUserAgentForURL(const URL& url)
 {
     auto quirks = UserAgentQuirks::quirksForURL(url);
     // The null string means we don't need a specific UA for the given URL.
+    // Note: UserAgentQuirks::NeedsUnbrandedUserAgent is implemented by simply
+    // not returning here.
     if (quirks.isEmpty())
         return String();
 

Tools/ChangeLog

@@ -1,3 +1,15 @@
+2021-03-10  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [WPE][GTK] Introduce NeedsUnbrandedUserAgent quirk and use it for accounts.google.com, docs.google.com, and drive.google.com
+        https://bugs.webkit.org/show_bug.cgi?id=222978
+
+        Reviewed by Carlos Garcia Campos.
+
+        * TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp:
+        (TestWebKitAPI::assertUserAgentForURLHasEmptyQuirk):
+        (TestWebKitAPI::TEST):
+        (TestWebKitAPI::assertUserAgentForURLHasLinuxPlatformQuirk): Deleted.
+
 2021-03-10  Aakash Jain  <aakash_jain@apple.com>
 
         [build.webkit.org] Update RunJavaScriptCoreTests step for new buildbot

Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp

@@ -54,17 +54,6 @@ static void assertUserAgentForURLHasFirefoxBrowserQuirk(const char* url)
     EXPECT_FALSE(uaString.contains("Version"));
 }
 
-static void assertUserAgentForURLHasLinuxPlatformQuirk(const char* url)
-{
-    String uaString = standardUserAgentForURL(URL({ }, url));
-
-    EXPECT_TRUE(uaString.contains("Linux"));
-    EXPECT_FALSE(uaString.contains("Macintosh"));
-    EXPECT_FALSE(uaString.contains("Mac OS X"));
-    EXPECT_FALSE(uaString.contains("Chrome"));
-    EXPECT_FALSE(uaString.contains("FreeBSD"));
-}
-
 static void assertUserAgentForURLHasMacPlatformQuirk(const char* url)
 {
     String uaString = standardUserAgentForURL(URL({ }, url));
@@ -76,36 +65,34 @@ static void assertUserAgentForURLHasMacPlatformQuirk(const char* url)
     EXPECT_FALSE(uaString.contains("FreeBSD"));
 }
 
+// Some Google domains require an unbranded user agent, which is a little
+// awkward to test for. We want to check that standardUserAgentForURL is
+// non-null to ensure *any* quirk URL is returned, so that application branding
+// does not get used. (standardUserAgentForURL returns a null string to indicate
+// that the standard user agent should be used.)
+static void assertUserAgentForURLHasEmptyQuirk(const char* url)
+{
+    String uaString = standardUserAgentForURL(URL({ }, url));
+    EXPECT_FALSE(uaString.isNull());
+}
+
 TEST(UserAgentTest, Quirks)
 {
     // A site with no quirks should return a null String.
     String uaString = standardUserAgentForURL(URL({ }, "http://www.webkit.org/"));
     EXPECT_TRUE(uaString.isNull());
 
-#if !OS(LINUX) || !CPU(X86_64)
-    // Google quirk should not affect sites with similar domains.
-    uaString = standardUserAgentForURL(URL({ }, "http://www.googleblog.com/"));
-    EXPECT_FALSE(uaString.contains("Linux x86_64"));
-#endif
-
     assertUserAgentForURLHasChromeBrowserQuirk("http://typekit.com/");
     assertUserAgentForURLHasChromeBrowserQuirk("http://typekit.net/");
     assertUserAgentForURLHasChromeBrowserQuirk("http://auth.mayohr.com/");
     assertUserAgentForURLHasChromeBrowserQuirk("http://bankofamerica.com/");
-    assertUserAgentForURLHasChromeBrowserQuirk("http://docs.google.com/");
 
     assertUserAgentForURLHasFirefoxBrowserQuirk("http://bugzilla.redhat.com/");
 
 #if ENABLE(THUNDER)
     assertUserAgentForURLHasFirefoxBrowserQuirk("http://www.netflix.com/");
 #endif
 
-    assertUserAgentForURLHasLinuxPlatformQuirk("http://www.google.com/");
-    assertUserAgentForURLHasLinuxPlatformQuirk("http://www.google.es/");
-    assertUserAgentForURLHasLinuxPlatformQuirk("http://calendar.google.com/");
-    assertUserAgentForURLHasLinuxPlatformQuirk("http://plus.google.com/");
-    assertUserAgentForURLHasLinuxPlatformQuirk("http://fonts.googleapis.com/");
-
     assertUserAgentForURLHasMacPlatformQuirk("http://www.yahoo.com/");
     assertUserAgentForURLHasMacPlatformQuirk("http://finance.yahoo.com/");
     assertUserAgentForURLHasMacPlatformQuirk("http://intl.taobao.com/");
@@ -116,6 +103,10 @@ TEST(UserAgentTest, Quirks)
     assertUserAgentForURLHasMacPlatformQuirk("http://outlook.office.com/");
     assertUserAgentForURLHasMacPlatformQuirk("http://mail.ntu.edu.tw/");
     assertUserAgentForURLHasMacPlatformQuirk("http://exchange.tu-berlin.de/");
+
+    assertUserAgentForURLHasEmptyQuirk("http://accounts.google.com/");
+    assertUserAgentForURLHasEmptyQuirk("http://docs.google.com/");
+    assertUserAgentForURLHasEmptyQuirk("http://drive.google.com/");
 }
 
 } // namespace TestWebKitAPI