Bug 163508

Summary: Move user agent quirks to cross-platform location
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebCore Misc.Assignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, commit-queue, mcatanzaro, olivier.blin
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=263619
Attachments:
Description Flags
Patch
none
Patch
none
Patch
cgarcia: review+, cgarcia: commit-queue-
Patch none

Description Michael Catanzaro 2016-10-16 12:53:09 PDT
A downstream port wants to use GTK's list of user agent quirks. It would surely be beneficial for EFL as well, and actually probably for pretty much any port except macOS and iOS (which are big enough that hopefully UA quirks are not needed).
Comment 1 Michael Catanzaro 2016-10-16 17:20:53 PDT
Created attachment 291787 [details]
Patch
Comment 2 Michael Catanzaro 2016-10-16 17:49:04 PDT
Created attachment 291788 [details]
Patch
Comment 3 Michael Catanzaro 2016-10-16 18:16:57 PDT
Created attachment 291789 [details]
Patch
Comment 4 Carlos Garcia Campos 2016-10-16 22:53:08 PDT
Comment on attachment 291789 [details]
Patch

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

Please don't do the "Standard" rename.

> Source/WebCore/ChangeLog:13
> +        * platform/StandardUserAgent.h: Renamed from Source/WebCore/platform/gtk/UserAgentGtk.h.
> +        Why not UserAgent.h? Because it clashes with a Cocoa-specific header, and would require
> +        editing XCode build files. Maybe a bad reason to rename a bunch of files, but there you go!

I know adding new files for mac is a pain, but I really think this should be called UserAgent.h. The cocoa file is in page/cocoa/ dir, and I don't think we have that in our include dir list, so it's probably safe to add it in platform. We can then file a sepàrate bug for the cocoa implementation to be merged in the new one, and we don't need to deal with Xcode in this bug.

> Source/WebCore/ChangeLog:29
> +        * platform/efl/StandardUserAgentEfl.cpp: Renamed from Source/WebCore/platform/efl/UserAgentEfl.cpp.
> +        (WebCore::standardUserAgentForURL):
> +        * platform/gtk/StandardUserAgentGtk.cpp: Renamed from Source/WebCore/platform/gtk/UserAgentGtk.cpp.

And then we don't need to rename these.
Comment 5 Michael Catanzaro 2016-10-17 02:58:37 PDT
(In reply to comment #4)
> Please don't do the "Standard" rename.

Yeah, this was dumb.

> I know adding new files for mac is a pain, but I really think this should be
> called UserAgent.h. The cocoa file is in page/cocoa/ dir, and I don't think
> we have that in our include dir list, so it's probably safe to add it in
> platform. We can then file a sepàrate bug for the cocoa implementation to be
> merged in the new one, and we don't need to deal with Xcode in this bug.

But cocoa ports do include Source/WebCore/platform. And the header files do get tracked in the XCode project files in several places unfortunately.

I should just make the necessary changes (remove page/cocoa/UserAgentCocoa.h and move page/cocoa/UserAgentCocoa.cpp to platform/cocoa) without touching the XCode files, and then we can ask someone on IRC into fixing up the XCode project files.
Comment 6 Michael Catanzaro 2016-10-17 03:45:00 PDT
Created attachment 291808 [details]
Patch
Comment 7 Michael Catanzaro 2016-10-17 04:31:33 PDT
Comment on attachment 291808 [details]
Patch

I played a trick with #include_next.
Comment 8 Michael Catanzaro 2016-10-17 04:32:03 PDT
Let's land this manually, EWS is choking on unrelated tests.
Comment 9 Michael Catanzaro 2016-10-17 04:33:02 PDT
Committed r207406: <http://trac.webkit.org/changeset/207406>