Bug 163508 - Move user agent quirks to cross-platform location
Summary: Move user agent quirks to cross-platform location
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-16 12:53 PDT by Michael Catanzaro
Modified: 2016-10-17 04:33 PDT (History)
4 users (show)

See Also:


Attachments
Patch (36.03 KB, patch)
2016-10-16 17:20 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (47.27 KB, patch)
2016-10-16 17:49 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (49.12 KB, patch)
2016-10-16 18:16 PDT, Michael Catanzaro
cgarcia: review+
cgarcia: commit-queue-
Details | Formatted Diff | Diff
Patch (33.77 KB, patch)
2016-10-17 03:45 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>