- Revision
- 210207
- Author
- mcatanz...@igalia.com
- Date
- 2016-12-30 01:02:51 -0800 (Fri, 30 Dec 2016)
Log Message
[GTK] Improve user agent construction
https://bugs.webkit.org/show_bug.cgi?id=142074
Reviewed by Carlos Garcia Campos.
Source/WebCore:
Using the macOS quirk rather than the Chrome quirk for Google domains was a mistake: it
broke Hangouts in a different way than the Chrome quirk, and also prevents use of the nice
Earth mode on Google Maps. Google is making it really hard to develop a sane quirk.
Eventually I settled on the combination of two quirks: (1) Firefox browser, and (2) Linux
x86_64 platform. See the bug for full discussion on why these quirks are the best way to
make Google domains work properly in WebKit. This is an extremely sad state of affairs, but
I'm confident it is the best option. Note this effectively includes a rollout of r210168.
Also, fix a bug that caused an extra space to be inserted in the middle of the user agent.
* platform/UserAgentQuirks.cpp:
(WebCore::isGoogle):
(WebCore::urlRequiresFirefoxBrowser):
(WebCore::urlRequiresMacintoshPlatform):
(WebCore::urlRequiresLinuxDesktopPlatform):
(WebCore::UserAgentQuirks::quirksForURL):
(WebCore::UserAgentQuirks::stringForQuirk):
(WebCore::UserAgentQuirks::firefoxRevisionString):
* platform/UserAgentQuirks.h:
* platform/gtk/UserAgentGtk.cpp:
(WebCore::buildUserAgentString):
Tools:
* TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp:
(TestWebKitAPI::assertUserAgentForURLHasChromeBrowserQuirk):
(TestWebKitAPI::assertUserAgentForURLHasFirefoxBrowserQuirk):
(TestWebKitAPI::assertUserAgentForURLHasLinuxPlatformQuirk):
(TestWebKitAPI::assertUserAgentForURLHasMacPlatformQuirk):
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (210206 => 210207)
--- trunk/Source/WebCore/ChangeLog 2016-12-30 08:54:25 UTC (rev 210206)
+++ trunk/Source/WebCore/ChangeLog 2016-12-30 09:02:51 UTC (rev 210207)
@@ -1,3 +1,32 @@
+2016-12-30 Michael Catanzaro <mcatanz...@igalia.com>
+
+ [GTK] Improve user agent construction
+ https://bugs.webkit.org/show_bug.cgi?id=142074
+
+ Reviewed by Carlos Garcia Campos.
+
+ Using the macOS quirk rather than the Chrome quirk for Google domains was a mistake: it
+ broke Hangouts in a different way than the Chrome quirk, and also prevents use of the nice
+ Earth mode on Google Maps. Google is making it really hard to develop a sane quirk.
+ Eventually I settled on the combination of two quirks: (1) Firefox browser, and (2) Linux
+ x86_64 platform. See the bug for full discussion on why these quirks are the best way to
+ make Google domains work properly in WebKit. This is an extremely sad state of affairs, but
+ I'm confident it is the best option. Note this effectively includes a rollout of r210168.
+
+ Also, fix a bug that caused an extra space to be inserted in the middle of the user agent.
+
+ * platform/UserAgentQuirks.cpp:
+ (WebCore::isGoogle):
+ (WebCore::urlRequiresFirefoxBrowser):
+ (WebCore::urlRequiresMacintoshPlatform):
+ (WebCore::urlRequiresLinuxDesktopPlatform):
+ (WebCore::UserAgentQuirks::quirksForURL):
+ (WebCore::UserAgentQuirks::stringForQuirk):
+ (WebCore::UserAgentQuirks::firefoxRevisionString):
+ * platform/UserAgentQuirks.h:
+ * platform/gtk/UserAgentGtk.cpp:
+ (WebCore::buildUserAgentString):
+
2016-12-30 Andreas Kling <akl...@apple.com>
Drop the render tree for documents in the page cache.
Modified: trunk/Source/WebCore/platform/UserAgentQuirks.cpp (210206 => 210207)
--- trunk/Source/WebCore/platform/UserAgentQuirks.cpp 2016-12-30 08:54:25 UTC (rev 210206)
+++ trunk/Source/WebCore/platform/UserAgentQuirks.cpp 2016-12-30 09:02:51 UTC (rev 210207)
@@ -34,6 +34,27 @@
// When editing the quirks in this file, be sure to update
// Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp.
+static bool isGoogle(const URL& url)
+{
+ String baseDomain = topPrivatelyControlledDomain(url.host());
+
+ // Our Google UA is *very* complicated to get right. Read
+ // https://webkit.org/b/142074 carefully before changing. Test that Earth
+ // view is available in Google Maps. Test Google Calendar. Test downloading
+ // the Hangouts browser plugin. Change platformVersionForUAString() to
+ // return "FreeBSD amd64" and test Maps and Calendar again.
+ if (baseDomain.startsWith("google."))
+ return true;
+ if (baseDomain == "gstatic.com")
+ return true;
+ if (baseDomain == "googleapis.com")
+ return true;
+ if (baseDomain == "googleusercontent.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)
@@ -56,15 +77,15 @@
return false;
}
+static bool urlRequiresFirefoxBrowser(const URL& url)
+{
+ return isGoogle(url);
+}
+
static bool urlRequiresMacintoshPlatform(const URL& url)
{
String baseDomain = topPrivatelyControlledDomain(url.host());
- // Avoid receiving terrible fallbacks version of calendar.google.com and
- // maps.google.com on certain platforms. https://webkit.org/b/142074
- if (baseDomain.startsWith("google."))
- return true;
-
// At least finance.yahoo.com displays a mobile version with WebKitGTK+'s standard user agent.
if (baseDomain == "yahoo.com")
return true;
@@ -80,14 +101,27 @@
return false;
}
+static bool urlRequiresLinuxDesktopPlatform(const URL& url)
+{
+ return isGoogle(url);
+}
+
UserAgentQuirks UserAgentQuirks::quirksForURL(const URL& url)
{
ASSERT(!url.isNull());
+
UserAgentQuirks quirks;
+
if (urlRequiresChromeBrowser(url))
quirks.add(UserAgentQuirks::NeedsChromeBrowser);
+ else if (urlRequiresFirefoxBrowser(url))
+ quirks.add(UserAgentQuirks::NeedsFirefoxBrowser);
+
if (urlRequiresMacintoshPlatform(url))
quirks.add(UserAgentQuirks::NeedsMacintoshPlatform);
+ else if (urlRequiresLinuxDesktopPlatform(url))
+ quirks.add(UserAgentQuirks::NeedsLinuxDesktopPlatform);
+
return quirks;
}
@@ -97,8 +131,13 @@
case NeedsChromeBrowser:
// Get versions from https://chromium.googlesource.com/chromium/src.git
return ASCIILiteral("Chrome/56.0.2891.4");
+ case NeedsFirefoxBrowser:
+ // Gecko version never changes. Firefox version must be updated below.
+ return ASCIILiteral("Gecko/20100101 Firefox/50.0");
case NeedsMacintoshPlatform:
return ASCIILiteral("Macintosh; Intel Mac OS X 10_12");
+ case NeedsLinuxDesktopPlatform:
+ return ASCIILiteral("X11; Linux x86_64");
case NumUserAgentQuirks:
default:
ASSERT_NOT_REACHED();
@@ -106,4 +145,9 @@
return ASCIILiteral("");
}
+String UserAgentQuirks::firefoxRevisionString()
+{
+ return ASCIILiteral("rv:50.0");
}
+
+}
Modified: trunk/Source/WebCore/platform/UserAgentQuirks.h (210206 => 210207)
--- trunk/Source/WebCore/platform/UserAgentQuirks.h 2016-12-30 08:54:25 UTC (rev 210206)
+++ trunk/Source/WebCore/platform/UserAgentQuirks.h 2016-12-30 09:02:51 UTC (rev 210207)
@@ -36,7 +36,9 @@
public:
enum UserAgentQuirk {
NeedsChromeBrowser,
+ NeedsFirefoxBrowser,
NeedsMacintoshPlatform,
+ NeedsLinuxDesktopPlatform,
NumUserAgentQuirks
};
@@ -66,6 +68,8 @@
static String stringForQuirk(UserAgentQuirk);
+ static String firefoxRevisionString();
+
private:
uint32_t m_quirks;
};
Modified: trunk/Source/WebCore/platform/gtk/UserAgentGtk.cpp (210206 => 210207)
--- trunk/Source/WebCore/platform/gtk/UserAgentGtk.cpp 2016-12-30 08:54:25 UTC (rev 210206)
+++ trunk/Source/WebCore/platform/gtk/UserAgentGtk.cpp 2016-12-30 09:02:51 UTC (rev 210207)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012, 2014 Igalia S.L.
+ * Copyright (C) 2012, 2014, 2016 Igalia S.L.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -82,6 +82,8 @@
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("; ");
@@ -88,20 +90,31 @@
uaString.append(platformVersionForUAString());
}
- uaString.appendLiteral(") AppleWebKit/");
- uaString.append(versionForUAString());
- uaString.appendLiteral(" (KHTML, like Gecko) ");
+ if (quirks.contains(UserAgentQuirks::NeedsFirefoxBrowser)) {
+ uaString.appendLiteral("; ");
+ uaString.append(UserAgentQuirks::firefoxRevisionString());
+ uaString.appendLiteral(") ");
+ } else {
+ uaString.appendLiteral(") AppleWebKit/");
+ uaString.append(versionForUAString());
+ uaString.appendLiteral(" (KHTML, like Gecko) ");
+ }
// Note that Chrome UAs advertise *both* Chrome and Safari.
if (quirks.contains(UserAgentQuirks::NeedsChromeBrowser)) {
uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsChromeBrowser));
uaString.appendLiteral(" ");
+ } else if (quirks.contains(UserAgentQuirks::NeedsFirefoxBrowser)) {
+ uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsFirefoxBrowser));
+ uaString.appendLiteral(" ");
}
- // Version/X is mandatory *before* Safari/X to be a valid Safari UA. See
- // https://bugs.webkit.org/show_bug.cgi?id=133403 for details.
- uaString.appendLiteral(" Version/10.0 Safari/");
- uaString.append(versionForUAString());
+ if (!quirks.contains(UserAgentQuirks::NeedsFirefoxBrowser)) {
+ // Version/X is mandatory *before* Safari/X to be a valid Safari UA. See
+ // https://bugs.webkit.org/show_bug.cgi?id=133403 for details.
+ uaString.appendLiteral("Version/10.0 Safari/");
+ uaString.append(versionForUAString());
+ }
return uaString.toString();
}
Modified: trunk/Tools/ChangeLog (210206 => 210207)
--- trunk/Tools/ChangeLog 2016-12-30 08:54:25 UTC (rev 210206)
+++ trunk/Tools/ChangeLog 2016-12-30 09:02:51 UTC (rev 210207)
@@ -1,3 +1,17 @@
+2016-12-30 Michael Catanzaro <mcatanz...@igalia.com>
+
+ [GTK] Improve user agent construction
+ https://bugs.webkit.org/show_bug.cgi?id=142074
+
+ Reviewed by Carlos Garcia Campos.
+
+ * TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp:
+ (TestWebKitAPI::assertUserAgentForURLHasChromeBrowserQuirk):
+ (TestWebKitAPI::assertUserAgentForURLHasFirefoxBrowserQuirk):
+ (TestWebKitAPI::assertUserAgentForURLHasLinuxPlatformQuirk):
+ (TestWebKitAPI::assertUserAgentForURLHasMacPlatformQuirk):
+ (TestWebKitAPI::TEST):
+
2016-12-27 Michael Catanzaro <mcatanz...@igalia.com>
[GTK] Improve user agent construction
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp (210206 => 210207)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp 2016-12-30 08:54:25 UTC (rev 210206)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp 2016-12-30 09:02:51 UTC (rev 210207)
@@ -36,22 +36,35 @@
{
String uaString = standardUserAgentForURL(URL(ParsedURLString, url));
-#if !OS(MAC_OS_X)
- EXPECT_FALSE(uaString.contains("Macintosh"));
- EXPECT_FALSE(uaString.contains("Mac OS X"));
-#endif
-#if OS(LINUX)
- EXPECT_TRUE(uaString.contains("Linux"));
-#endif
-#if OS(WINDOWS)
- EXPECT_TRUE(uaString.contains("Windows"));
-#endif
-
EXPECT_TRUE(uaString.contains("Chrome"));
EXPECT_TRUE(uaString.contains("Safari"));
EXPECT_FALSE(uaString.contains("Chromium"));
+ EXPECT_FALSE(uaString.contains("Firefox"));
}
+static void assertUserAgentForURLHasFirefoxBrowserQuirk(const char* url)
+{
+ String uaString = standardUserAgentForURL(URL(ParsedURLString, url));
+
+ EXPECT_FALSE(uaString.contains("Chrome"));
+ EXPECT_FALSE(uaString.contains("Safari"));
+ EXPECT_FALSE(uaString.contains("Chromium"));
+ EXPECT_FALSE(uaString.contains("AppleWebKit"));
+ EXPECT_TRUE(uaString.contains("Firefox"));
+}
+
+static void assertUserAgentForURLHasLinuxPlatformQuirk(const char* url)
+{
+ String uaString = standardUserAgentForURL(URL(ParsedURLString, url));
+
+ EXPECT_TRUE(uaString.contains("Linux"));
+ EXPECT_FALSE(uaString.contains("Macintosh"));
+ EXPECT_FALSE(uaString.contains("Mac OS X"));
+ EXPECT_FALSE(uaString.contains("Windows"));
+ EXPECT_FALSE(uaString.contains("Chrome"));
+ EXPECT_FALSE(uaString.contains("FreeBSD"));
+}
+
static void assertUserAgentForURLHasMacPlatformQuirk(const char* url)
{
String uaString = standardUserAgentForURL(URL(ParsedURLString, url));
@@ -61,6 +74,7 @@
EXPECT_FALSE(uaString.contains("Linux"));
EXPECT_FALSE(uaString.contains("Windows"));
EXPECT_FALSE(uaString.contains("Chrome"));
+ EXPECT_FALSE(uaString.contains("FreeBSD"));
}
TEST(UserAgentTest, Quirks)
@@ -69,12 +83,9 @@
String uaString = standardUserAgentForURL(URL(ParsedURLString, "http://www.webkit.org/"));
EXPECT_TRUE(uaString.isNull());
-#if !OS(MAC_OS_X)
// Google quirk should not affect sites with similar domains.
uaString = standardUserAgentForURL(URL(ParsedURLString, "http://www.googleblog.com/"));
- EXPECT_FALSE(uaString.contains("Macintosh"));
- EXPECT_FALSE(uaString.contains("Mac OS X"));
-#endif
+ EXPECT_FALSE(uaString.contains("Firefox"));
assertUserAgentForURLHasChromeBrowserQuirk("http://typekit.com/");
assertUserAgentForURLHasChromeBrowserQuirk("http://typekit.net/");
@@ -81,17 +92,21 @@
assertUserAgentForURLHasChromeBrowserQuirk("http://www.youtube.com/");
assertUserAgentForURLHasChromeBrowserQuirk("http://www.slack.com/");
+ assertUserAgentForURLHasFirefoxBrowserQuirk("http://www.google.com/");
+ assertUserAgentForURLHasFirefoxBrowserQuirk("http://www.google.es/");
+ assertUserAgentForURLHasFirefoxBrowserQuirk("http://calendar.google.com/");
+ assertUserAgentForURLHasFirefoxBrowserQuirk("http://plus.google.com/");
+
+ assertUserAgentForURLHasLinuxPlatformQuirk("http://www.google.com/");
+ assertUserAgentForURLHasLinuxPlatformQuirk("http://www.google.es/");
+ assertUserAgentForURLHasLinuxPlatformQuirk("http://calendar.google.com/");
+ assertUserAgentForURLHasLinuxPlatformQuirk("http://plus.google.com/");
+
assertUserAgentForURLHasMacPlatformQuirk("http://www.yahoo.com/");
assertUserAgentForURLHasMacPlatformQuirk("http://finance.yahoo.com/");
assertUserAgentForURLHasMacPlatformQuirk("http://intl.taobao.com/");
assertUserAgentForURLHasMacPlatformQuirk("http://www.whatsapp.com/");
assertUserAgentForURLHasMacPlatformQuirk("http://web.whatsapp.com/");
-
- assertUserAgentForURLHasMacPlatformQuirk("http://www.google.com/");
- assertUserAgentForURLHasMacPlatformQuirk("http://www.google.es/");
- assertUserAgentForURLHasMacPlatformQuirk("http://calendar.google.com/");
- assertUserAgentForURLHasMacPlatformQuirk("http://maps.google.com/");
- assertUserAgentForURLHasMacPlatformQuirk("http://plus.google.com/");
}
} // namespace TestWebKitAPI
