Bug 196642

Summary: [GTK][WPE] outlook.live.com displays old-fashioned UI
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: WebKitGTKAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Sergio Villar Senin 2019-04-05 02:39:54 PDT
Outlook email (former hotmail.com) displays a very old interface whenever logging in with WebKitGtk browsers. It isn't a matter of configuration on the server side because with the very same credentials I can see the new look and feel in any other browser.

Looks like we need to adjust the UA for those domains.
Comment 1 Sergio Villar Senin 2019-04-05 03:22:54 PDT
Indeed adding a Chrome string just before Safari, for example "Chrome/72.0.3626.81" makes it show the new UI instead of the clunky old one.
Comment 2 Sergio Villar Senin 2019-04-05 03:46:31 PDT
Patch coming soon...
Comment 3 Sergio Villar Senin 2019-04-05 04:26:54 PDT
Created attachment 366808 [details]
Patch
Comment 4 Michael Catanzaro 2019-04-05 08:28:22 PDT
Comment on attachment 366808 [details]
Patch

Can't you use the Mac platform quirk instead?

    // Microsoft Outlook Web App forces users with WebKitGTK+'s standard user
    // agent to use the light version. Earlier versions even blocks users from
    // accessing the calendar.
    if (domain == "outlook.live.com" || domain == "mail.ntu.edu.tw")
        return true;

Sadly there are a bazillion hosts running this service and we can't whitelist them all, but we can whitelist the main one at least.
Comment 5 Sergio Villar Senin 2019-04-07 12:24:42 PDT
(In reply to Michael Catanzaro from comment #4)
> Comment on attachment 366808 [details]
> Patch
> 
> Can't you use the Mac platform quirk instead?
> 
>     // Microsoft Outlook Web App forces users with WebKitGTK+'s standard user
>     // agent to use the light version. Earlier versions even blocks users
> from
>     // accessing the calendar.
>     if (domain == "outlook.live.com" || domain == "mail.ntu.edu.tw")
>         return true;
> 
> Sadly there are a bazillion hosts running this service and we can't
> whitelist them all, but we can whitelist the main one at least.

Any reason in particular why we prefer to use a different arch instead of adding the Chrome UA? Is it just because we want the server to identify us as a WebKit browser instead of a Chromium one?
Comment 6 Michael Catanzaro 2019-04-07 12:41:19 PDT
It's a safer quirk, yes. With a Chrome quirk there's greater risk of breakage if the site starts using Chrome-specific JS in the future, so the macOS quirk should be preferred whenever possible. The Chrome quirk has caused several problems in the past so it should only be used if there are no better options.
Comment 7 Sergio Villar Senin 2019-04-07 15:47:39 PDT
Created attachment 366908 [details]
Patch
Comment 8 Carlos Garcia Campos 2019-04-08 01:13:14 PDT
Comment on attachment 366908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366908&action=review

> Source/WebCore/platform/UserAgentQuirks.cpp:64
>  {
> -    String baseDomain = topPrivatelyControlledDomain(url.host().toString());
> +    String domain = url.host().toString();
> +    String baseDomain = topPrivatelyControlledDomain(domain);

This is not needed.
Comment 9 Sergio Villar Senin 2019-04-08 02:04:04 PDT
Comment on attachment 366908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366908&action=review

>> Source/WebCore/platform/UserAgentQuirks.cpp:64
>> +    String baseDomain = topPrivatelyControlledDomain(domain);
> 
> This is not needed.

Right, forgot to delete it
Comment 10 Sergio Villar Senin 2019-04-08 02:06:46 PDT
Created attachment 366922 [details]
Patch
Comment 11 Sergio Villar Senin 2019-04-08 02:09:09 PDT
Committed r243971: <https://trac.webkit.org/changeset/243971>
Comment 12 Sergio Villar Senin 2019-04-25 03:11:17 PDT
Comment on attachment 366922 [details]
Patch

Removing r? as the patch already landed