- Revision
- 274210
- Author
- [email protected]
- Date
- 2021-03-10 06:29:47 -0800 (Wed, 10 Mar 2021)
Log Message
[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
Patch by Michael Catanzaro <[email protected]> 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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (274209 => 274210)
--- trunk/Source/WebCore/ChangeLog 2021-03-10 12:52:34 UTC (rev 274209)
+++ trunk/Source/WebCore/ChangeLog 2021-03-10 14:29:47 UTC (rev 274210)
@@ -1,3 +1,40 @@
+2021-03-10 Michael Catanzaro <[email protected]>
+
+ [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 <[email protected]>
Unreviewed, reverting r274166.
Modified: trunk/Source/WebCore/platform/UserAgentQuirks.cpp (274209 => 274210)
--- trunk/Source/WebCore/platform/UserAgentQuirks.cpp 2021-03-10 12:52:34 UTC (rev 274209)
+++ trunk/Source/WebCore/platform/UserAgentQuirks.cpp 2021-03-10 14:29:47 UTC (rev 274210)
@@ -34,38 +34,14 @@
// When editing the quirks in this file, be sure to update
// Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp.
+//
+// When testing changes, be sure to test with application branding enabled.
+// Otherwise, we will not notice when urlRequiresUnbrandedUserAgent is needed.
-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;
-}
-
// 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 @@
// 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 @@
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,9 +118,24 @@
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)
@@ -156,18 +142,21 @@
{
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 @@
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;
Modified: trunk/Source/WebCore/platform/UserAgentQuirks.h (274209 => 274210)
--- trunk/Source/WebCore/platform/UserAgentQuirks.h 2021-03-10 12:52:34 UTC (rev 274209)
+++ trunk/Source/WebCore/platform/UserAgentQuirks.h 2021-03-10 14:29:47 UTC (rev 274210)
@@ -35,10 +35,8 @@
enum UserAgentQuirk {
NeedsChromeBrowser,
NeedsFirefoxBrowser,
- NeedsInternetExplorerBrowser,
NeedsMacintoshPlatform,
- NeedsWindowsPlatform,
- NeedsLinuxDesktopPlatform,
+ NeedsUnbrandedUserAgent,
NumUserAgentQuirks
};
Modified: trunk/Source/WebCore/platform/glib/UserAgentGLib.cpp (274209 => 274210)
--- trunk/Source/WebCore/platform/glib/UserAgentGLib.cpp 2021-03-10 12:52:34 UTC (rev 274209)
+++ trunk/Source/WebCore/platform/glib/UserAgentGLib.cpp 2021-03-10 14:29:47 UTC (rev 274210)
@@ -75,12 +75,6 @@
#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 @@
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 @@
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 @@
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 @@
} else {
String finalApplicationVersion = applicationVersion;
if (finalApplicationVersion.isEmpty())
- finalApplicationVersion = versionForUAString();
+ finalApplicationVersion = "605.1.15";
userAgent = standardUserAgentStatic() + ' ' + applicationName + '/' + finalApplicationVersion;
}
@@ -169,6 +158,8 @@
{
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();
Modified: trunk/Tools/ChangeLog (274209 => 274210)
--- trunk/Tools/ChangeLog 2021-03-10 12:52:34 UTC (rev 274209)
+++ trunk/Tools/ChangeLog 2021-03-10 14:29:47 UTC (rev 274210)
@@ -1,3 +1,15 @@
+2021-03-10 Michael Catanzaro <[email protected]>
+
+ [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 <[email protected]>
[build.webkit.org] Update RunJavaScriptCoreTests step for new buildbot
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp (274209 => 274210)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp 2021-03-10 12:52:34 UTC (rev 274209)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp 2021-03-10 14:29:47 UTC (rev 274210)
@@ -54,17 +54,6 @@
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,6 +65,17 @@
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.
@@ -82,17 +82,10 @@
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/");
@@ -100,12 +93,6 @@
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 @@
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
