Bug 133403

Summary: [GTK] Unsupported browser in www.icloud.com
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: WebKitGTKAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, cgarcia, mjs, mrobinson, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 132681, 133855    
Attachments:
Description Flags
Patch
none
Patch mrobinson: review+, cgarcia: commit-queue-

Description Sergio Villar Senin 2014-05-30 02:29:22 PDT
Just to keep track of broken sites.
Comment 1 Sergio Villar Senin 2014-06-11 08:20:25 PDT
So the problem is basically that in the UA string we need to add the "Version/X" thing, and this is very important, *before* the "Safari/X" part, otherwise it won't be recognized as a valid browser UA.

Note that according to http://www.useragentstring.com/pages/Safari/, "Version/X" is always located before "Safari/X" since it was introduced in Safari v3.0.
Comment 2 Sergio Villar Senin 2014-06-11 09:31:00 PDT
Created attachment 232870 [details]
Patch
Comment 3 Martin Robinson 2014-06-11 09:50:25 PDT
Comment on attachment 232870 [details]
Patch

Yeah, omitting the version is pretty bogus. We just need to keep it up to date. Perhaps we can make it equal the current version of Safari which better matches our feature profile.
Comment 4 Sergio Villar Senin 2014-06-11 10:54:51 PDT
(In reply to comment #3)
> (From update of attachment 232870 [details])
> Yeah, omitting the version is pretty bogus. We just need to keep it up to date. Perhaps we can make it equal the current version of Safari which better matches our feature profile.

For sure, I tried to be a bit conservative and left 6.0. It was released with Mountain Lion back in 2012, but we might consider switching to Safari 7/6.1 which was released last year for Mavericks, Lion and Mountain Lion. We should be fine. WDYT?
Comment 5 Martin Robinson 2014-06-11 11:07:07 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 232870 [details] [details])
> > Yeah, omitting the version is pretty bogus. We just need to keep it up to date. Perhaps we can make it equal the current version of Safari which better matches our feature profile.
> 
> For sure, I tried to be a bit conservative and left 6.0. It was released with Mountain Lion back in 2012, but we might consider switching to Safari 7/6.1 which was released last year for Mavericks, Lion and Mountain Lion. We should be fine. WDYT?

I think we should go with 8 (or 7 if that doesn't work). 8 should have WebGL turned on by default.
Comment 6 Sergio Villar Senin 2014-06-11 12:39:49 PDT
Created attachment 232889 [details]
Patch

Claim to be Safari 8
Comment 7 Martin Robinson 2014-06-11 12:57:28 PDT
Comment on attachment 232889 [details]
Patch

Let's give it a shot. We can always back down to 7, if we need.
Comment 8 Carlos Garcia Campos 2014-06-12 00:02:42 PDT
Comment on attachment 232889 [details]
Patch

Please, update the unit test before landing. See Tools/TestWebKitAPI/Tests/WebCore/gtk/UserAgentQuirks.cpp
Comment 9 Carlos Garcia Campos 2014-06-12 00:07:20 PDT
Comment on attachment 232889 [details]
Patch

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

Please, update the unit test before landing. See Tools/TestWebKitAPI/Tests/WebCore/gtk/UserAgentQuirks.cpp

> Source/WebCore/platform/gtk/UserAgentGtk.cpp:141
> -    uaString.appendLiteral(" (KHTML, like Gecko) Safari/");
> +    uaString.appendLiteral(" (KHTML, like Gecko) Version/8.0 Safari/");

Could you also add a comment here including the URL of this bug, please?
Comment 10 Sergio Villar Senin 2014-06-12 02:26:12 PDT
Committed r169887: <http://trac.webkit.org/changeset/169887>