[webkit-reviews] review denied: [Bug 119584] [Gtk] URL printing code in DumpRenderTree doesn't match WTR or Mac DRT : [Attachment 208732] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 14 17:42:44 PDT 2013


Gustavo Noronha (kov) <gns at gnome.org> has denied Simon Pena
<simon.pena at samsung.com>'s request for review:
Bug 119584: [Gtk] URL printing code in DumpRenderTree doesn't match WTR or Mac
DRT
https://bugs.webkit.org/show_bug.cgi?id=119584

Attachment 208732: Patch
https://bugs.webkit.org/attachment.cgi?id=208732&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=208732&action=review


r- because of the suggested changes, and I think it would be great to include
the fix for the password you mentioned, too

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:83
> +static const char divider = '/';

I don't think it's necessary to use a const name just for a single usage, I'd
use '/' directly, not like this is making the code more readable.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1151
>      if (g_str_equal(uri->scheme, "http") || g_str_equal(uri->scheme, "ftp"))
{

This should be a check for !file scheme instead of http || ftp, according to
ap. It's what EFL is doing as well, we should probably follow.


More information about the webkit-reviews mailing list