Bug 175507

Summary: [WPE] Implement WebCore::standardUserAgent()
Product: WebKit Reporter: Adrian Perez <aperez>
Component: WPE WebKitAssignee: Adrian Perez <aperez>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, bugs-noreply, cgarcia, clopez, commit-queue, mcatanzaro, zan
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Adrian Perez 2017-08-12 01:00:59 PDT
There are a couple of FIXMEs in the code “UserAgentGtk.cpp”:

  #if PLATFORM(GTK)
     CString newUserAgent = WebCore::standardUserAgent(String::fromUTF8(applicationName), String::fromUTF8(applicationVersion)).utf8();
     webkit_settings_set_user_agent(settings, newUserAgent.data());
  #elif PLATFORM(WPE)
     // FIXME: WPE should implement WebCore::standardUserAgent.
  #endif

Also, this file should be renamed to “UserAgentGLib.cpp”, like the
rest of the sources used by both the WebKitGTK+ and the WPE ports.
Comment 1 Adrian Perez 2017-08-12 01:08:40 PDT
Created attachment 317988 [details]
Patch
Comment 2 WebKit Commit Bot 2017-08-13 05:13:31 PDT
Comment on attachment 317988 [details]
Patch

Clearing flags on attachment: 317988

Committed r220631: <http://trac.webkit.org/changeset/220631>
Comment 3 WebKit Commit Bot 2017-08-13 05:13:32 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Carlos Garcia Campos 2017-08-13 22:42:04 PDT
(In reply to Adrian Perez from comment #0)
> There are a couple of FIXMEs in the code “UserAgentGtk.cpp”:
> 
>   #if PLATFORM(GTK)
>      CString newUserAgent =
> WebCore::standardUserAgent(String::fromUTF8(applicationName),
> String::fromUTF8(applicationVersion)).utf8();
>      webkit_settings_set_user_agent(settings, newUserAgent.data());
>   #elif PLATFORM(WPE)
>      // FIXME: WPE should implement WebCore::standardUserAgent.
>   #endif
> 
> Also, this file should be renamed to “UserAgentGLib.cpp”, like the
> rest of the sources used by both the WebKitGTK+ and the WPE ports.

This is not true/accurate, sometimes we use other suffix like Unix, Linux, ... or not suffix at all but other platforms don't compile the file. In this particulat case it doesn't make sense to use GLib, it was called Gtk not because it uses GTK+, but because it was the implementation of the GTK port. But there isn't a glib port, and the file doesn't use glib at all. This is only used by ports expose the GLib API, though, so we can probably leave it now that the patch landed already.
Comment 5 Carlos Garcia Campos 2017-08-13 22:43:36 PDT
Comment on attachment 317988 [details]
Patch

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

> Source/cmake/OptionsGTK.cmake:198
> +add_definitions(-DUSER_AGENT_GLIB_MAJOR_VERSION="604")
> +add_definitions(-DUSER_AGENT_GLIB_MINOR_VERSION="1")

This is confusing, again here GTK meant the GTK port, so the UA version used by the GTK port. I think we can simply remove the GLIB and use USER_AGENT_MAJOR|MINOR_VERSION

> Source/cmake/OptionsWPE.cmake:130
> +add_definitions(-DUSER_AGENT_GLIB_MAJOR_VERSION="601")
> +add_definitions(-DUSER_AGENT_GLIB_MINOR_VERSION="1")

Ditto.
Comment 6 Michael Catanzaro 2017-08-14 08:24:33 PDT
Reopening, please address Carlos's comments!
Comment 7 Adrian Perez 2017-08-14 09:38:27 PDT
ACK, I'll post a follow-up removing the “_GTK” bit from the defines.
I understand that's enought, and it is _not_ needed to rename
“UserAgentGLib.cpp” to something else.
Comment 8 Adrian Perez 2017-08-14 09:39:57 PDT
(In reply to Adrian Perez from comment #7)
> ACK, I'll post a follow-up removing the “_GTK” bit from the defines.
> I understand that's enought, and it is _not_ needed to rename
> “UserAgentGLib.cpp” to something else.

I meant: “_GLIB” removed.
Comment 9 Adrian Perez 2017-08-14 09:44:33 PDT
Created attachment 318042 [details]
Patch
Comment 10 WebKit Commit Bot 2017-08-14 11:42:29 PDT
Comment on attachment 318042 [details]
Patch

Clearing flags on attachment: 318042

Committed r220713: <http://trac.webkit.org/changeset/220713>
Comment 11 WebKit Commit Bot 2017-08-14 11:42:30 PDT
All reviewed patches have been landed.  Closing bug.