Changeset 210207 in webkit


Ignore:
Timestamp:
Dec 30, 2016 1:02:51 AM (7 years ago)
Author:
Michael Catanzaro
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):

Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r210206 r210207  
     12016-12-30  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        [GTK] Improve user agent construction
     4        https://bugs.webkit.org/show_bug.cgi?id=142074
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        Using the macOS quirk rather than the Chrome quirk for Google domains was a mistake: it
     9        broke Hangouts in a different way than the Chrome quirk, and also prevents use of the nice
     10        Earth mode on Google Maps. Google is making it really hard to develop a sane quirk.
     11        Eventually I settled on the combination of two quirks: (1) Firefox browser, and (2) Linux
     12        x86_64 platform. See the bug for full discussion on why these quirks are the best way to
     13        make Google domains work properly in WebKit. This is an extremely sad state of affairs, but
     14        I'm confident it is the best option. Note this effectively includes a rollout of r210168.
     15
     16        Also, fix a bug that caused an extra space to be inserted in the middle of the user agent.
     17
     18        * platform/UserAgentQuirks.cpp:
     19        (WebCore::isGoogle):
     20        (WebCore::urlRequiresFirefoxBrowser):
     21        (WebCore::urlRequiresMacintoshPlatform):
     22        (WebCore::urlRequiresLinuxDesktopPlatform):
     23        (WebCore::UserAgentQuirks::quirksForURL):
     24        (WebCore::UserAgentQuirks::stringForQuirk):
     25        (WebCore::UserAgentQuirks::firefoxRevisionString):
     26        * platform/UserAgentQuirks.h:
     27        * platform/gtk/UserAgentGtk.cpp:
     28        (WebCore::buildUserAgentString):
     29
    1302016-12-30  Andreas Kling  <akling@apple.com>
    231

  • trunk/Source/WebCore/platform/UserAgentQuirks.cpp

    r210168 r210207  
    3535// Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp.
    3636
     37static bool isGoogle(const URL& url)
     38{
     39    String baseDomain = topPrivatelyControlledDomain(url.host());
     40
     41    // Our Google UA is *very* complicated to get right. Read
     42    // https://webkit.org/b/142074 carefully before changing. Test that Earth
     43    // view is available in Google Maps. Test Google Calendar. Test downloading
     44    // the Hangouts browser plugin. Change platformVersionForUAString() to
     45    // return "FreeBSD amd64" and test Maps and Calendar again.
     46    if (baseDomain.startsWith("google."))
     47        return true;
     48    if (baseDomain == "gstatic.com")
     49        return true;
     50    if (baseDomain == "googleapis.com")
     51        return true;
     52    if (baseDomain == "googleusercontent.com")
     53        return true;
     54
     55    return false;
     56}
     57
    3758// Be careful with this quirk: it's an invitation for sites to use JavaScript
    3859// that works in Chrome that WebKit cannot handle. Prefer other quirks instead.
    … …  
    5778}
    5879
     80static bool urlRequiresFirefoxBrowser(const URL& url)
     81{
     82    return isGoogle(url);
     83}
     84
    5985static bool urlRequiresMacintoshPlatform(const URL& url)
    6086{
    6187    String baseDomain = topPrivatelyControlledDomain(url.host());
    62 
    63     // Avoid receiving terrible fallbacks version of calendar.google.com and
    64     // maps.google.com on certain platforms. https://webkit.org/b/142074
    65     if (baseDomain.startsWith("google."))
    66         return true;
    6788
    6889    // At least finance.yahoo.com displays a mobile version with WebKitGTK+'s standard user agent.
    … …  
    81102}
    82103
     104static bool urlRequiresLinuxDesktopPlatform(const URL& url)
     105{
     106    return isGoogle(url);
     107}
     108
    83109UserAgentQuirks UserAgentQuirks::quirksForURL(const URL& url)
    84110{
    85111    ASSERT(!url.isNull());
     112
    86113    UserAgentQuirks quirks;
     114
    87115    if (urlRequiresChromeBrowser(url))
    88116        quirks.add(UserAgentQuirks::NeedsChromeBrowser);
     117    else if (urlRequiresFirefoxBrowser(url))
     118        quirks.add(UserAgentQuirks::NeedsFirefoxBrowser);
     119
    89120    if (urlRequiresMacintoshPlatform(url))
    90121        quirks.add(UserAgentQuirks::NeedsMacintoshPlatform);
     122    else if (urlRequiresLinuxDesktopPlatform(url))
     123        quirks.add(UserAgentQuirks::NeedsLinuxDesktopPlatform);
     124
    91125    return quirks;
    92126}
    … …  
    98132        // Get versions from https://chromium.googlesource.com/chromium/src.git
    99133        return ASCIILiteral("Chrome/56.0.2891.4");
     134    case NeedsFirefoxBrowser:
     135        // Gecko version never changes. Firefox version must be updated below.
     136        return ASCIILiteral("Gecko/20100101 Firefox/50.0");
    100137    case NeedsMacintoshPlatform:
    101138        return ASCIILiteral("Macintosh; Intel Mac OS X 10_12");
     139    case NeedsLinuxDesktopPlatform:
     140        return ASCIILiteral("X11; Linux x86_64");
    102141    case NumUserAgentQuirks:
    103142    default:
    … …  
    107146}
    108147
     148String UserAgentQuirks::firefoxRevisionString()
     149{
     150    return ASCIILiteral("rv:50.0");
    109151}
     152
     153}

  • trunk/Source/WebCore/platform/UserAgentQuirks.h

    r207406 r210207  
    3737    enum UserAgentQuirk {
    3838        NeedsChromeBrowser,
     39        NeedsFirefoxBrowser,
    3940        NeedsMacintoshPlatform,
     41        NeedsLinuxDesktopPlatform,
    4042
    4143        NumUserAgentQuirks
    … …  
    6769    static String stringForQuirk(UserAgentQuirk);
    6870
     71    static String firefoxRevisionString();
     72
    6973private:
    7074    uint32_t m_quirks;

  • trunk/Source/WebCore/platform/gtk/UserAgentGtk.cpp

    r207406 r210207  
    11/*
    2  * Copyright (C) 2012, 2014 Igalia S.L.
     2 * Copyright (C) 2012, 2014, 2016 Igalia S.L.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    8383    if (quirks.contains(UserAgentQuirks::NeedsMacintoshPlatform))
    8484        uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsMacintoshPlatform));
     85    else if (quirks.contains(UserAgentQuirks::NeedsLinuxDesktopPlatform))
     86        uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsLinuxDesktopPlatform));
    8587    else {
    8688        uaString.append(platformForUAString());
    … …  
    8991    }
    9092
    91     uaString.appendLiteral(") AppleWebKit/");
    92     uaString.append(versionForUAString());
    93     uaString.appendLiteral(" (KHTML, like Gecko) ");
     93    if (quirks.contains(UserAgentQuirks::NeedsFirefoxBrowser)) {
     94        uaString.appendLiteral("; ");
     95        uaString.append(UserAgentQuirks::firefoxRevisionString());
     96        uaString.appendLiteral(") ");
     97    } else {
     98        uaString.appendLiteral(") AppleWebKit/");
     99        uaString.append(versionForUAString());
     100        uaString.appendLiteral(" (KHTML, like Gecko) ");
     101    }
    94102
    95103    // Note that Chrome UAs advertise *both* Chrome and Safari.
    … …  
    97105        uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsChromeBrowser));
    98106        uaString.appendLiteral(" ");
     107    } else if (quirks.contains(UserAgentQuirks::NeedsFirefoxBrowser)) {
     108        uaString.append(UserAgentQuirks::stringForQuirk(UserAgentQuirks::NeedsFirefoxBrowser));
     109        uaString.appendLiteral(" ");
    99110    }
    100111
    101     // Version/X is mandatory *before* Safari/X to be a valid Safari UA. See
    102     // https://bugs.webkit.org/show_bug.cgi?id=133403 for details.
    103     uaString.appendLiteral(" Version/10.0 Safari/");
    104     uaString.append(versionForUAString());
     112    if (!quirks.contains(UserAgentQuirks::NeedsFirefoxBrowser)) {
     113        // Version/X is mandatory *before* Safari/X to be a valid Safari UA. See
     114        // https://bugs.webkit.org/show_bug.cgi?id=133403 for details.
     115        uaString.appendLiteral("Version/10.0 Safari/");
     116        uaString.append(versionForUAString());
     117    }
    105118
    106119    return uaString.toString();

  • trunk/Tools/ChangeLog

    r210168 r210207  
     12016-12-30  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        [GTK] Improve user agent construction
     4        https://bugs.webkit.org/show_bug.cgi?id=142074
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        * TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp:
     9        (TestWebKitAPI::assertUserAgentForURLHasChromeBrowserQuirk):
     10        (TestWebKitAPI::assertUserAgentForURLHasFirefoxBrowserQuirk):
     11        (TestWebKitAPI::assertUserAgentForURLHasLinuxPlatformQuirk):
     12        (TestWebKitAPI::assertUserAgentForURLHasMacPlatformQuirk):
     13        (TestWebKitAPI::TEST):
     14
    1152016-12-27  Michael Catanzaro  <mcatanzaro@igalia.com>
    216

  • trunk/Tools/TestWebKitAPI/Tests/WebCore/UserAgentQuirks.cpp

    r210168 r210207  
    3737    String uaString = standardUserAgentForURL(URL(ParsedURLString, url));
    3838
    39 #if !OS(MAC_OS_X)
    40     EXPECT_FALSE(uaString.contains("Macintosh"));
    41     EXPECT_FALSE(uaString.contains("Mac OS X"));
    42 #endif
    43 #if OS(LINUX)
    44     EXPECT_TRUE(uaString.contains("Linux"));
    45 #endif
    46 #if OS(WINDOWS)
    47     EXPECT_TRUE(uaString.contains("Windows"));
    48 #endif
    49 
    5039    EXPECT_TRUE(uaString.contains("Chrome"));
    5140    EXPECT_TRUE(uaString.contains("Safari"));
    5241    EXPECT_FALSE(uaString.contains("Chromium"));
     42    EXPECT_FALSE(uaString.contains("Firefox"));
     43}
     44
     45static void assertUserAgentForURLHasFirefoxBrowserQuirk(const char* url)
     46{
     47    String uaString = standardUserAgentForURL(URL(ParsedURLString, url));
     48
     49    EXPECT_FALSE(uaString.contains("Chrome"));
     50    EXPECT_FALSE(uaString.contains("Safari"));
     51    EXPECT_FALSE(uaString.contains("Chromium"));
     52    EXPECT_FALSE(uaString.contains("AppleWebKit"));
     53    EXPECT_TRUE(uaString.contains("Firefox"));
     54}
     55
     56static void assertUserAgentForURLHasLinuxPlatformQuirk(const char* url)
     57{
     58    String uaString = standardUserAgentForURL(URL(ParsedURLString, url));
     59
     60    EXPECT_TRUE(uaString.contains("Linux"));
     61    EXPECT_FALSE(uaString.contains("Macintosh"));
     62    EXPECT_FALSE(uaString.contains("Mac OS X"));
     63    EXPECT_FALSE(uaString.contains("Windows"));
     64    EXPECT_FALSE(uaString.contains("Chrome"));
     65    EXPECT_FALSE(uaString.contains("FreeBSD"));
    5366}
    5467
    … …  
    6275    EXPECT_FALSE(uaString.contains("Windows"));
    6376    EXPECT_FALSE(uaString.contains("Chrome"));
     77    EXPECT_FALSE(uaString.contains("FreeBSD"));
    6478}
    6579
    … …  
    7084    EXPECT_TRUE(uaString.isNull());
    7185
    72 #if !OS(MAC_OS_X)
    7386    // Google quirk should not affect sites with similar domains.
    7487    uaString = standardUserAgentForURL(URL(ParsedURLString, "http://www.googleblog.com/"));
    75     EXPECT_FALSE(uaString.contains("Macintosh"));
    76     EXPECT_FALSE(uaString.contains("Mac OS X"));
    77 #endif
     88    EXPECT_FALSE(uaString.contains("Firefox"));
    7889
    7990    assertUserAgentForURLHasChromeBrowserQuirk("http://typekit.com/");
    … …  
    8293    assertUserAgentForURLHasChromeBrowserQuirk("http://www.slack.com/");
    8394
     95    assertUserAgentForURLHasFirefoxBrowserQuirk("http://www.google.com/");
     96    assertUserAgentForURLHasFirefoxBrowserQuirk("http://www.google.es/");
     97    assertUserAgentForURLHasFirefoxBrowserQuirk("http://calendar.google.com/");
     98    assertUserAgentForURLHasFirefoxBrowserQuirk("http://plus.google.com/");
     99
     100    assertUserAgentForURLHasLinuxPlatformQuirk("http://www.google.com/");
     101    assertUserAgentForURLHasLinuxPlatformQuirk("http://www.google.es/");
     102    assertUserAgentForURLHasLinuxPlatformQuirk("http://calendar.google.com/");
     103    assertUserAgentForURLHasLinuxPlatformQuirk("http://plus.google.com/");
     104
    84105    assertUserAgentForURLHasMacPlatformQuirk("http://www.yahoo.com/");
    85106    assertUserAgentForURLHasMacPlatformQuirk("http://finance.yahoo.com/");
    … …  
    87108    assertUserAgentForURLHasMacPlatformQuirk("http://www.whatsapp.com/");
    88109    assertUserAgentForURLHasMacPlatformQuirk("http://web.whatsapp.com/");
    89 
    90     assertUserAgentForURLHasMacPlatformQuirk("http://www.google.com/");
    91     assertUserAgentForURLHasMacPlatformQuirk("http://www.google.es/");
    92     assertUserAgentForURLHasMacPlatformQuirk("http://calendar.google.com/");
    93     assertUserAgentForURLHasMacPlatformQuirk("http://maps.google.com/");
    94     assertUserAgentForURLHasMacPlatformQuirk("http://plus.google.com/");
    95110}
    96111
Note: See TracChangeset for help on using the changeset viewer.