Merge r270168 - [WPE][GTK] Use Internet Explorer quirk for Google Docs

https://bugs.webkit.org/show_bug.cgi?id=219278 Patch by Michael Catanzaro <mcatanzaro@gnome.org> on 2020-11-26 Reviewed by Carlos Garcia Campos. Source/WebCore: Since r266584, we've suffered from an annoying unsupported browser warning when using Google Docs. We don't have many options to avoid it. I'm afraid that Firefox or Chrome quirks are too risky, since these seem to tempt Google into using web platform features that WebKit does not support. The safest quirk is the macOS platform quirk, but that doesn't work well here because it breaks various keyboard shortcuts like Ctrl+A and Ctrl+Z. So an Internet Explorer quirk is really the last card we have left. I think this is the safest Google quirk we've ever had, in that it's pretty unlikely that Google will try to send Internet Explorer anything that doesn't work in WebKit. However, it will break eventually, whenever Google decides that Internet Explorer is no longer supported. I guess that date is probably at least five years away, so we can only hope that Google drops this anticompetitive nonsense before then. P.S. Let's also switch Google Drive to this new quirk. The unsupported browser warning on Google Drive seems to be completely independent, but the IE quirk seems safer that the existing Firefox quirk, so why not? * platform/UserAgentQuirks.cpp: (WebCore::isGoogle): (WebCore::urlRequiresInternetExplorerBrowser): (WebCore::urlRequiresWindowsPlatform): (WebCore::urlRequiresLinuxDesktopPlatform): (WebCore::UserAgentQuirks::quirksForURL): (WebCore::UserAgentQuirks::stringForQuirk): * platform/UserAgentQuirks.h: * platform/glib/UserAgentGLib.cpp: (WebCore::buildUserAgentString): Tools: * TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp: (TestWebKitAPI::assertUserAgentForURLHasChromeBrowserQuirk): (TestWebKitAPI::assertUserAgentForURLHasFirefoxBrowserQuirk): (TestWebKitAPI::assertUserAgentForURLHasInternetExplorerBrowserQuirk): (TestWebKitAPI::assertUserAgentForURLHasWindowsPlatformQuirk): (TestWebKitAPI::TEST):
WebKit · Feb 1, 2021 · dfff805 · dfff805
1 parent 2d97056
commit dfff805
Show file tree

Hide file tree

Showing 6 changed files with 131 additions and 15 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,38 @@
+2020-11-26  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [WPE][GTK] Use Internet Explorer quirk for Google Docs
+        https://bugs.webkit.org/show_bug.cgi?id=219278
+
+        Reviewed by Carlos Garcia Campos.
+
+        Since r266584, we've suffered from an annoying unsupported browser warning when using Google
+        Docs. We don't have many options to avoid it. I'm afraid that Firefox or Chrome quirks are
+        too risky, since these seem to tempt Google into using web platform features that WebKit
+        does not support. The safest quirk is the macOS platform quirk, but that doesn't work well
+        here because it breaks various keyboard shortcuts like Ctrl+A and Ctrl+Z. So an Internet
+        Explorer quirk is really the last card we have left.
+
+        I think this is the safest Google quirk we've ever had, in that it's pretty unlikely that
+        Google will try to send Internet Explorer anything that doesn't work in WebKit. However, it
+        will break eventually, whenever Google decides that Internet Explorer is no longer
+        supported. I guess that date is probably at least five years away, so we can only hope that
+        Google drops this anticompetitive nonsense before then.
+
+        P.S. Let's also switch Google Drive to this new quirk. The unsupported browser warning on
+        Google Drive seems to be completely independent, but the IE quirk seems safer that the
+        existing Firefox quirk, so why not?
+
+        * platform/UserAgentQuirks.cpp:
+        (WebCore::isGoogle):
+        (WebCore::urlRequiresInternetExplorerBrowser):
+        (WebCore::urlRequiresWindowsPlatform):
+        (WebCore::urlRequiresLinuxDesktopPlatform):
+        (WebCore::UserAgentQuirks::quirksForURL):
+        (WebCore::UserAgentQuirks::stringForQuirk):
+        * platform/UserAgentQuirks.h:
+        * platform/glib/UserAgentGLib.cpp:
+        (WebCore::buildUserAgentString):
+
 2020-12-11  Miguel Gomez  <magomez@igalia.com>
 
         REGRESSION(r268923): [WPE] Nothing renders on the rpi3 using the proprietary video driver

diff --git a/Source/WebCore/platform/UserAgentQuirks.cpp b/Source/WebCore/platform/UserAgentQuirks.cpp
@@ -38,11 +38,6 @@ namespace WebCore {
 static bool isGoogle(const URL& url)
 {
     String domain = url.host().toString();
-
-    // Google uses accounts.youtube.com for its login service.
-    if (domain == "accounts.youtube.com")
-        return true;
-
     String baseDomain = topPrivatelyControlledDomain(domain);
 
     // Our Google UA is *very* complicated to get right. Read
@@ -98,11 +93,6 @@ static bool urlRequiresFirefoxBrowser(const URL& url)
 {
     String domain = url.host().toString();
 
-    // Google Drive shows an unsupported browser warning with WebKitGTK's
-    // standard user agent.
-    if (domain == "drive.google.com")
-        return true;
-
     // Red Hat Bugzilla displays a warning page when performing searches with WebKitGTK's standard
     // user agent.
     if (domain == "bugzilla.redhat.com")
@@ -111,6 +101,31 @@ static bool urlRequiresFirefoxBrowser(const URL& url)
     return false;
 }
 
+static bool urlRequiresInternetExplorerBrowser(const URL& url)
+{
+    String domain = url.host().toString();
+
+    // This quirk actually has nothing to do with YouTube. It's needed to avoid
+    // unsupported browser warnings on Google Docs. Why do we need this weird
+    // quirk? We cannot use Chrome or Firefox quirks because Google then uses
+    // features that don't work in WebKit. And we can't use our macOS platform
+    // quirk because Google then expects command keys and doesn't support basic
+    // keyboard shortcuts. We could pretend to be Edge, but adding Chromium is
+    // likely to break in the same way as a Chrome quirk. So that leaves us
+    // with IE browser as the final reasonable option. This will break
+    // eventually, but hopefully not for a long time, because we are probably
+    // out of options when it does.
+    if (domain == "accounts.youtube.com" || domain == "docs.google.com")
+        return true;
+
+    // Google Drive shows an unsupported browser warning with WebKitGTK's
+    // standard user agent.
+    if (domain == "drive.google.com")
+        return true;
+
+    return false;
+}
+
 static bool urlRequiresMacintoshPlatform(const URL& url)
 {
     String domain = url.host().toString();
@@ -148,9 +163,14 @@ static bool urlRequiresMacintoshPlatform(const URL& url)
     return false;
 }
 
+static bool urlRequiresWindowsPlatform(const URL& url)
+{
+    return urlRequiresInternetExplorerBrowser(url);
+}
+
 static bool urlRequiresLinuxDesktopPlatform(const URL& url)
 {
-    return isGoogle(url) && chassisType() != WTF::ChassisType::Mobile;
+    return isGoogle(url) && !urlRequiresInternetExplorerBrowser(url) && chassisType() != WTF::ChassisType::Mobile;
 }
 
 UserAgentQuirks UserAgentQuirks::quirksForURL(const URL& url)
@@ -163,9 +183,13 @@ UserAgentQuirks UserAgentQuirks::quirksForURL(const URL& url)
         quirks.add(UserAgentQuirks::NeedsChromeBrowser);
     else if (urlRequiresFirefoxBrowser(url))
         quirks.add(UserAgentQuirks::NeedsFirefoxBrowser);
+    else if (urlRequiresInternetExplorerBrowser(url))
+        quirks.add(UserAgentQuirks::NeedsInternetExplorerBrowser);
 
     if (urlRequiresMacintoshPlatform(url))
         quirks.add(UserAgentQuirks::NeedsMacintoshPlatform);
+    else if (urlRequiresWindowsPlatform(url))
+        quirks.add(UserAgentQuirks::NeedsWindowsPlatform);
     else if (urlRequiresLinuxDesktopPlatform(url))
         quirks.add(UserAgentQuirks::NeedsLinuxDesktopPlatform);
 
@@ -180,8 +204,12 @@ String UserAgentQuirks::stringForQuirk(UserAgentQuirk quirk)
         return "Chrome/86.0.4208.2"_s;
     case NeedsFirefoxBrowser:
         return "; rv:80.0) Gecko/20100101 Firefox/80.0"_s;
+    case NeedsInternetExplorerBrowser:
+        return "; Trident/7.0; rv:11.0) like Gecko"_s;
     case NeedsMacintoshPlatform:
         return "Macintosh; Intel Mac OS X 10_15"_s;
+    case NeedsWindowsPlatform:
+        return "Windows NT 10.0"_s;
     case NeedsLinuxDesktopPlatform:
         return "X11; Linux x86_64"_s;
     case NumUserAgentQuirks:

diff --git a/Source/WebCore/platform/UserAgentQuirks.h b/Source/WebCore/platform/UserAgentQuirks.h
@@ -35,7 +35,9 @@ class UserAgentQuirks {
     enum UserAgentQuirk {
         NeedsChromeBrowser,
         NeedsFirefoxBrowser,
+        NeedsInternetExplorerBrowser,
         NeedsMacintoshPlatform,
+        NeedsWindowsPlatform,
         NeedsLinuxDesktopPlatform,
 
         NumUserAgentQuirks

diff --git a/Source/WebCore/platform/glib/UserAgentGLib.cpp b/Source/WebCore/platform/glib/UserAgentGLib.cpp
@@ -89,6 +89,8 @@ static String buildUserAgentString(const UserAgentQuirks& quirks)
 
     if (quirks.contains(UserAgentQuirks::NeedsMacintoshPlatform))
         uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsMacintoshPlatform));
+    else if (quirks.contains(UserAgentQuirks::NeedsWindowsPlatform))
+        uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsWindowsPlatform));
     else if (quirks.contains(UserAgentQuirks::NeedsLinuxDesktopPlatform))
         uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsLinuxDesktopPlatform));
     else {
@@ -104,6 +106,10 @@ static String buildUserAgentString(const UserAgentQuirks& quirks)
         uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsFirefoxBrowser));
         return uaString.toString();
     }
+    if (quirks.contains(UserAgentQuirks::NeedsInternetExplorerBrowser)) {
+        uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsInternetExplorerBrowser));
+        return uaString.toString();
+    }
 
     uaString.appendLiteral(") AppleWebKit/");
     uaString.append(versionForUAString());

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
@@ -1,3 +1,17 @@
+2020-11-26  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [WPE][GTK] Use Internet Explorer quirk for Google Docs
+        https://bugs.webkit.org/show_bug.cgi?id=219278
+
+        Reviewed by Carlos Garcia Campos.
+
+        * TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp:
+        (TestWebKitAPI::assertUserAgentForURLHasChromeBrowserQuirk):
+        (TestWebKitAPI::assertUserAgentForURLHasFirefoxBrowserQuirk):
+        (TestWebKitAPI::assertUserAgentForURLHasInternetExplorerBrowserQuirk):
+        (TestWebKitAPI::assertUserAgentForURLHasWindowsPlatformQuirk):
+        (TestWebKitAPI::TEST):
+
 2020-12-07  Zan Dobersek  <zdobersek@igalia.com>
 
         [GLib] Leaked RunLoop objects on worker threads

diff --git a/Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp b/Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp
@@ -41,6 +41,7 @@ static void assertUserAgentForURLHasChromeBrowserQuirk(const char* url)
     EXPECT_FALSE(uaString.contains("Chromium"));
     EXPECT_FALSE(uaString.contains("Firefox"));
     EXPECT_FALSE(uaString.contains("Version"));
+    EXPECT_FALSE(uaString.contains("Trident"));
 }
 
 static void assertUserAgentForURLHasFirefoxBrowserQuirk(const char* url)
@@ -52,6 +53,19 @@ static void assertUserAgentForURLHasFirefoxBrowserQuirk(const char* url)
     EXPECT_FALSE(uaString.contains("Chromium"));
     EXPECT_TRUE(uaString.contains("Firefox"));
     EXPECT_FALSE(uaString.contains("Version"));
+    EXPECT_FALSE(uaString.contains("Trident"));
+}
+
+static void assertUserAgentForURLHasInternetExplorerBrowserQuirk(const char* url)
+{
+    String uaString = standardUserAgentForURL(URL({ }, url));
+
+    EXPECT_FALSE(uaString.contains("Chrome"));
+    EXPECT_FALSE(uaString.contains("Safari"));
+    EXPECT_FALSE(uaString.contains("Chromium"));
+    EXPECT_FALSE(uaString.contains("Firefox"));
+    EXPECT_FALSE(uaString.contains("Version"));
+    EXPECT_TRUE(uaString.contains("Trident"));
 }
 
 static void assertUserAgentForURLHasLinuxPlatformQuirk(const char* url)
@@ -66,6 +80,18 @@ static void assertUserAgentForURLHasLinuxPlatformQuirk(const char* url)
     EXPECT_FALSE(uaString.contains("FreeBSD"));
 }
 
+static void assertUserAgentForURLHasWindowsPlatformQuirk(const char* url)
+{
+    String uaString = standardUserAgentForURL(URL({ }, url));
+
+    EXPECT_FALSE(uaString.contains("Macintosh"));
+    EXPECT_FALSE(uaString.contains("Mac OS X"));
+    EXPECT_FALSE(uaString.contains("Linux"));
+    EXPECT_TRUE(uaString.contains("Windows"));
+    EXPECT_FALSE(uaString.contains("Chrome"));
+    EXPECT_FALSE(uaString.contains("FreeBSD"));
+}
+
 static void assertUserAgentForURLHasMacPlatformQuirk(const char* url)
 {
     String uaString = standardUserAgentForURL(URL({ }, url));
@@ -80,7 +106,7 @@ static void assertUserAgentForURLHasMacPlatformQuirk(const char* url)
 
 TEST(UserAgentTest, Quirks)
 {
-    // A site with not quirks should return a null String.
+    // A site with no quirks should return a null String.
     String uaString = standardUserAgentForURL(URL({ }, "http://www.webkit.org/"));
     EXPECT_TRUE(uaString.isNull());
 
@@ -95,17 +121,22 @@ TEST(UserAgentTest, Quirks)
     assertUserAgentForURLHasChromeBrowserQuirk("http://auth.mayohr.com/");
     assertUserAgentForURLHasChromeBrowserQuirk("http://bankofamerica.com/");
 
-    assertUserAgentForURLHasFirefoxBrowserQuirk("http://drive.google.com/");
     assertUserAgentForURLHasFirefoxBrowserQuirk("http://bugzilla.redhat.com/");
 
+    assertUserAgentForURLHasInternetExplorerBrowserQuirk("http://accounts.youtube.com/");
+    assertUserAgentForURLHasInternetExplorerBrowserQuirk("http://docs.google.com/");
+    assertUserAgentForURLHasInternetExplorerBrowserQuirk("http://drive.google.com/");
+
+    assertUserAgentForURLHasWindowsPlatformQuirk("http://accounts.youtube.com/");
+    assertUserAgentForURLHasWindowsPlatformQuirk("http://docs.google.com/");
+    assertUserAgentForURLHasWindowsPlatformQuirk("http://drive.google.com/");
+
     assertUserAgentForURLHasLinuxPlatformQuirk("http://www.google.com/");
     assertUserAgentForURLHasLinuxPlatformQuirk("http://www.google.es/");
     assertUserAgentForURLHasLinuxPlatformQuirk("http://calendar.google.com/");
     assertUserAgentForURLHasLinuxPlatformQuirk("http://plus.google.com/");
     assertUserAgentForURLHasLinuxPlatformQuirk("http://drive.google.com/");
     assertUserAgentForURLHasLinuxPlatformQuirk("http://fonts.googleapis.com/");
-    assertUserAgentForURLHasLinuxPlatformQuirk("http://accounts.youtube.com/");
-    assertUserAgentForURLHasLinuxPlatformQuirk("http://docs.google.com/");
 
     assertUserAgentForURLHasMacPlatformQuirk("http://www.yahoo.com/");
     assertUserAgentForURLHasMacPlatformQuirk("http://finance.yahoo.com/");