Bug 73104 – [EFL] Remove duplicated UA information function

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

[Gyuyoung Kim]

Though ewk_setting already implemented an internal function for UA(user agent) information, FrameLoaderClientEfl has used duplicated function for UA information.

[Gyuyoung Kim]

Created attachment 116554 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 116554 [details]
Patch

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

> Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:78
> +    return makeString(ewk_settings_default_user_agent_get());

makeString has been deprecated for some time, please either create the string directly or return ewk_settings_default_user_agent_get() and let the String object be implicitly created.

> Source/WebKit/efl/ewk/ewk_settings.cpp:-72
> -

I'd rather leave this empty line here for readability.

> Source/WebKit/efl/ewk/ewk_settings.cpp:-77
> -

Ditto.

[Gyuyoung Kim]

Created attachment 116653 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 116554 [details]
Patch

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

>> Source/WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp:78
>> +    return makeString(ewk_settings_default_user_agent_get());
> 
> makeString has been deprecated for some time, please either create the string directly or return ewk_settings_default_user_agent_get() and let the String object be implicitly created.

Done. I use String directly.

>> Source/WebKit/efl/ewk/ewk_settings.cpp:-72
>> -
> 
> I'd rather leave this empty line here for readability.

Done.

>> Source/WebKit/efl/ewk/ewk_settings.cpp:-77
>> -
> 
> Ditto.

I think it is better to remove empty line on #elif.

[Raphael Kubo da Costa (:rakuco)]

LGTM.

[Filip Pizlo]

Comment on attachment 116653 [details]
Patch

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

r=me

> Source/WebKit/efl/ewk/ewk_settings.cpp:80
> +    uaOsVersion = "PPC Mac OS X";

Slight nitpick: presumably CPU(X86) tells you that you were *built* on X86.  So you could be running a PPC binary on an X86 Mac.  You could query what kind of Mac you're on using sysctl().  hw.machine will tell you the kind of CPU.  This is not a big deal, probably.

[Gyuyoung Kim]

(In reply to comment #6)
> (From update of attachment 116653 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=116653&action=review
> 
> r=me
> 
> > Source/WebKit/efl/ewk/ewk_settings.cpp:80
> > +    uaOsVersion = "PPC Mac OS X";
> 
> Slight nitpick: presumably CPU(X86) tells you that you were *built* on X86.  So you could be running a PPC binary on an X86 Mac.  You could query what kind of Mac you're on using sysctl().  hw.machine will tell you the kind of CPU.  This is not a big deal, probably.

Ok, I will enhance this function according to your advice on new bug. Thank you.

[WebKit Review Bot]

Comment on attachment 116653 [details]
Patch

Clearing flags on attachment: 116653

Committed r101471: <http://trac.webkit.org/changeset/101471>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.