[webkit-reviews] review denied: [Bug 36227] [GTK] Failing tests http/tests/misc/image-blocked-src-change.html & http/tests/misc/image-blocked-src-no-change.html : [Attachment 50922] Patch that modifies the way we print local URIs in DRT

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied Sergio Villar Senin
<svillar at igalia.com>'s request for review:
Bug 36227: [GTK] Failing tests http/tests/misc/image-blocked-src-change.html &
http/tests/misc/image-blocked-src-no-change.html
https://bugs.webkit.org/show_bug.cgi?id=36227

Attachment 50922: Patch that modifies the way we print local URIs in DRT
https://bugs.webkit.org/attachment.cgi?id=50922&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
>+    gchar* testMessage = NULL;

Let's try to use 0 instead of NULL, since it's what the style guide says (yes,
I realize this file in particular has a mix of NULLs and 0s...)

>+    const gchar* uriScheme;
>+
>+    // Tests expect only the filename part of local URIs
>+    uriScheme = g_strstr_len(message, -1, "file://");
>+    if (uriScheme) {
>+	  GString* tempString = g_string_sized_new(strlen(message));
>+	  gchar* filename = g_strrstr(uriScheme, G_DIR_SEPARATOR_S);
>+
>+	  if (filename) {
>+	      filename++;
>+	      tempString = g_string_append_len(tempString, message, (uriScheme
- message));
>+	      tempString = g_string_append_len(tempString, filename,
strlen(filename));
>+	      testMessage = g_string_free(tempString, FALSE);
>+	  }

You are assuming that there's only one file:// and that it will be the last
thing in the string, right? The first assumption seems to be shared by all
DRTs, but not the second one, so perhaps you should find the last separator
going forward from file:// ? You could just jump to the first whitespace and
then either do it manually or use g_path_get_basename, I guess.

>+    }
>+
>+    fprintf(stdout, "CONSOLE MESSAGE: line %d: %s\n", line, (testMessage) ?
testMessage : message);

The parenthesis around testMessage are not really needed.

>+    g_free(testMessage);
>+
>     return TRUE;
> }
> 
>-- 
>1.7.0
>