Merge lp:~tuxator/midori/colors into lp:midori

[Paweł Forysiuk]

Split colorful tabs code into helper functions and add unit tests

[Cris Dywan (kalikiana)]

I love the refactoring.

As a follow-up thought, it might be nice to improve the big if () before the color function calls, it duplicates part of what the functions do and probably can be improved.

review: Approve

[Cris Dywan (kalikiana)]

If I'm nitpicking I'd use a prefix for the function names in case you see them in a backtrace but that can also be fixed another time.

review: Approve

=== modified file 'extensions/colorful-tabs.c'
--- extensions/colorful-tabs.c	2013-04-26 18:54:16 +0000
+++ extensions/colorful-tabs.c	2013-05-27 10:54:30 +0000
@@ -13,55 +13,90 @@
 #include <midori/midori.h>
 
 static GdkColor
-view_get_bgcolor_for_hostname (MidoriView* view, gchar* hostname)
-{
-    GdkColor color;
-    GdkPixbuf* icon = midori_view_get_icon (view);
-    if (icon != NULL)
-    {
-        GdkPixbuf* newpix;
-        guchar* pixels;
-
-        newpix = gdk_pixbuf_scale_simple (icon, 1, 1, GDK_INTERP_BILINEAR);
-        pixels = gdk_pixbuf_get_pixels (newpix);
-        color.red = pixels[0] * 255;
-        color.green = pixels[1] * 255;
-        color.blue = pixels[2] * 255;
-    }
-    else
-    {
-        gchar* hash, *colorstr;
-
-        hash = g_compute_checksum_for_string (G_CHECKSUM_MD5, hostname, 1);
-        colorstr = g_strndup (hash, 6 + 1);
-        colorstr[0] = '#';
-        gdk_color_parse (colorstr, &color);
-
-        g_free (hash);
-        g_free (colorstr);
-    }
-
-    if ((color.red   < 35000)
-     && (color.green < 35000)
-     && (color.blue  < 35000))
-    {
-        color.red   += 20000;
-        color.green += 20000;
-        color.blue  += 20000;
-    }
-
-    if (color.red < 10000)
-        color.red = 5000;
-    else
-        color.red -= 5000;
-    if (color.blue < 10000)
-        color.blue = 5000;
-    else
-        color.blue -= 5000;
-    if (color.green < 10000)
-        color.green = 5000;
-    else
-        color.green -= 5000;
+get_foreground_color_for_GdkColor (GdkColor color)
+{
+    /* rgb (160, 160, 160) is gray */
+    guint gray = 160 * 255;
+    GdkColor fgcolor;
+
+    /* Ensure high contrast by enforcing black/ white text colour. */
+    if ((color.red < gray)
+    && (color.green < gray)
+    && (color.blue < gray))
+        gdk_color_parse ("white", &fgcolor);
+    else
+        gdk_color_parse ("black", &fgcolor);
+
+    return fgcolor;
+}
+
+static GdkColor adjust_brightness (GdkColor color)
+{
+    guint dark_grey = 137 * 255;
+    guint adjustment = 78 * 255;
+    guint blue = 39 * 255;
+    guint readjust = 19 * 255;
+
+    if ((color.red   < dark_grey)
+     && (color.green < dark_grey)
+     && (color.blue  < dark_grey))
+    {
+        color.red   += adjustment;
+        color.green += adjustment;
+        color.blue  += adjustment;
+    }
+
+    if (color.red < blue)
+        color.red = readjust;
+    else
+        color.red -= readjust;
+
+    if (color.blue < blue)
+        color.blue = readjust;
+    else
+        color.blue -= readjust;
+
+    if (color.green < blue)
+        color.green = readjust;
+    else
+        color.green -= readjust;
+
+    return color;
+}
+
+static GdkColor
+view_get_bgcolor_for_favicon (GdkPixbuf* icon)
+{
+    GdkColor color;
+    GdkPixbuf* newpix;
+    guchar* pixels;
+
+    newpix = gdk_pixbuf_scale_simple (icon, 1, 1, GDK_INTERP_BILINEAR);
+    pixels = gdk_pixbuf_get_pixels (newpix);
+    color.red = pixels[0] * 255;
+    color.green = pixels[1] * 255;
+    color.blue = pixels[2] * 255;
+
+    color = adjust_brightness (color);
+
+    return color;
+}
+
+static GdkColor
+view_get_bgcolor_for_hostname (gchar* hostname)
+{
+    gchar* hash, *colorstr;
+    GdkColor color;
+
+    hash = g_compute_checksum_for_string (G_CHECKSUM_MD5, hostname, 1);
+    colorstr = g_strndup (hash, 6 + 1);
+    colorstr[0] = '#';
+    gdk_color_parse (colorstr, &color);
+
+    g_free (hash);
+    g_free (colorstr);
+
+    color = adjust_brightness (color);
 
     return color;
 }
@@ -71,25 +106,28 @@
                                   GParamSpec*      pspec,
                                   MidoriExtension* extension)
 {
-    gchar* hostname;
-    GdkColor color;
-    GdkColor fgcolor;
+    const gchar* uri = midori_view_get_display_uri (view);
+    if (!*uri)
+        return;
 
-    if (!midori_uri_is_blank (midori_view_get_display_uri (view))
-      && (hostname = midori_uri_parse_hostname (midori_view_get_display_uri (view), NULL))
-      && midori_view_get_icon_uri (view) != NULL)
+    if (!midori_uri_is_blank (uri))
     {
-        color = view_get_bgcolor_for_hostname (view, hostname);
-        g_free (hostname);
-        /* Ensure high contrast by enforcing black/ white text colour. */
-        if ((color.red   < 41000)
-         && (color.green < 41000)
-         && (color.blue  < 41000))
-            gdk_color_parse ("#fff", &fgcolor);
-        else
-            gdk_color_parse ("#000", &fgcolor);
-
-        midori_view_set_colors (view, &fgcolor, &color);
+        gchar* hostname = midori_uri_parse_hostname (uri, NULL);
+        if (hostname)
+        {
+            GdkColor fgcolor, color;
+            GdkPixbuf* icon = midori_view_get_icon (view);
+
+            if (icon)
+                color = view_get_bgcolor_for_favicon (icon);
+            else
+                color = view_get_bgcolor_for_hostname (hostname);
+
+            fgcolor = get_foreground_color_for_GdkColor (color);
+            midori_view_set_colors (view, &fgcolor, &color);
+
+            g_free (hostname);
+        }
     }
     else
         midori_view_set_colors (view, NULL, NULL);
@@ -170,6 +208,51 @@
     g_object_unref (browsers);
 }
 
+void test_colour_for_hostname (void)
+{
+    GdkColor color;
+    GdkColor fgcolor;
+
+    typedef struct
+    {
+        const gchar* host;
+        const gchar* fgcolor;
+        const gchar* color;
+    } ColorItem;
+
+    static const ColorItem items[] = {
+     { "www.last.fm", "#ffffffffffff", "#12ed7da312ed" },
+     { "git.xfce.org", "#000000000000", "#1c424c72e207" },
+     { "elementaryos.org", "#000000000000", "#50dbac36b43e" },
+     { "news.ycombinator.com", "#000000000000", "#a5cba6cc5278" },
+     { "cgit.freedesktop.org", "#ffffffffffff", "#95bb8db37ca2" },
+     { "get.cm", "#000000000000", "#1c424c72e207" },
+    };
+
+    guint i;
+    for (i = 0; i < G_N_ELEMENTS (items); i++)
+    {
+        color = view_get_bgcolor_for_hostname ((gchar*)items[i].host);
+        fgcolor = get_foreground_color_for_GdkColor (color);
+
+        g_assert_cmpstr (items[i].color, ==, gdk_color_to_string (&color));
+        g_assert_cmpstr (items[i].fgcolor, ==, gdk_color_to_string (&fgcolor));
+    }
+}
+
+void
+extension_test (void)
+{
+    #ifndef HAVE_WEBKIT2
+    g_object_set_data (G_OBJECT (webkit_get_default_session ()),
+                       "midori-session-initialized", (void*)1);
+    #endif
+
+    /* TODO: Add test which uses favicon codepath */
+
+    g_test_add_func ("/extensions/colorful_tabs/hostname_colour", test_colour_for_hostname);
+}
+
 MidoriExtension*
 extension_init (void)
 {