Bug 175507 – [WPE] Implement WebCore::standardUserAgent()

• Status: RESOLVED
• Resolution: FIXED
• Component: WPE WebKit

[Adrian Perez]

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.

[Adrian Perez]

Created attachment 317988 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 317988 [details]
Patch

Clearing flags on attachment: 317988

Committed r220631: <http://trac.webkit.org/changeset/220631>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Carlos Garcia Campos]

(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.

[Carlos Garcia Campos]

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.

[Michael Catanzaro]

Reopening, please address Carlos's comments!

[Adrian Perez]

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.

[Adrian Perez]

(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.

[Adrian Perez]

Created attachment 318042 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 318042 [details]
Patch

Clearing flags on attachment: 318042

Committed r220713: <http://trac.webkit.org/changeset/220713>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.