Bug 17626 – Programatically set background color and 16-bit alpha on WebKit GTK

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit API

[Sean Egan]

The OSX API allows you to set a background NSColor, which represents and RGBA value. The GTK+ analog to this is GdkColor, which only supports RGB. I've modeled this interface after GtkColorSelection which has separate, but similar, functions and properties for GdkColors and a separate guint16 alpha value.

I've kept the set_transparent/get_transparent API for backward compatibility, but suspect they should be deprecated.


I also fixed some compiler warnings.

[Sean Egan]

Created attachment 19469 [details]
Patch to set background color and alpha

[Alp Toker]

(In reply to comment #0)
> The OSX API allows you to set a background NSColor, which represents and RGBA
> value. The GTK+ analog to this is GdkColor, which only supports RGB. I've
> modeled this interface after GtkColorSelection which has separate, but similar,
> functions and properties for GdkColors and a separate guint16 alpha value.
> 
> I've kept the set_transparent/get_transparent API for backward compatibility,
> but suspect they should be deprecated.

set_transparent/get_transparent() is actually based directly on the drawsBackground from the Mac API (there might still be a case for removing it), but you are right that setting a background color is also a useful feature present on Mac.

I'll need to compare the proposed implementation to the one in Mac since Mac's was slightly more compilicated IIRC.

Out of curiosity, is the background color/alpha setting useful in apps (guessing this is for pidgin)?

[Sean Egan]

(In reply to comment #2)

> set_transparent/get_transparent() is actually based directly on the
> drawsBackground from the Mac API (there might still be a case for removing it),
> but you are right that setting a background color is also a useful feature
> present on Mac.

Oh, ok. I misundersood what 'setDrawsBackground' does, but it looks like maybe it's supposed to be deprecated? Adium says:

        if ([self respondsToSelector:@selector(setBackgroundColor:)]) {
                //As of Safari 3.0, we must call setBackgroundColor: to make the webview transparent
                [self setBackgroundColor:(flag ? [NSColor clearColor] : [NSColor whiteColor])];

        } else {
                [self setDrawsBackground:!flag];
        }

> Out of curiosity, is the background color/alpha setting useful in apps
> (guessing this is for pidgin)?
 
Yeah! http://pidgin.im/~seanegan/webkit11.png

[Darin Adler]

Comment on attachment 19469 [details]
Patch to set background color and alpha

+        g_value_set_uint(value, webkit_web_view_get_background_alpha(webView));
     default:

Missing a break here. I'm going to set review- because of that. Otherwise looks pretty good, but I'm not a GTK platform expert.

-        gboolean transparent;
+	GdkColor bg_color;
+	guint16 bg_alpha;
+	gboolean bg_color_set;

This indentation in the new code is done with tabs, and there's also a tab in the ChangeLog entry. We can't check tabs into our repository, so please try to avoid tabs while writing the patch to help the person committing it.

[Sean Egan]

Created attachment 19655 [details]
New version of patch

[Holger Freyther]

(In reply to comment #5)
> Created an attachment (id=19655) [edit]
> New version of patch
> 

+    case PROP_BG_ALPHA:
+        webkit_web_view_set_background_alpha(webView, g_value_get_uint(value));
     default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);

another missing break?

[Holger Freyther]

Some minor coding style issues, the int cast should probably be a different patch but if alp is happy with the API one can sort this out when landing the patch.

But please try to follow the Coding Style it makes it more easy for us to land patches.

[Holger Freyther]

Okay, I will merge this.

[Holger Freyther]

Comment on attachment 19655 [details]
New version of patch

Okay.

[Holger Freyther]

Okay, I landed the patch in r32723 but there are a couple of things:
   - I added the missing break
   - I removed reading the bg property (also removed the method) because bg_color was unitialized or contained old data. e.g. a call to set_bg_color(NULL) and then asking for the color would still return the old color.
   - You want to set the transparency for the view() in WebKit/gtk/WebCoreSupport/FrameLoaderClient as well.

Please send an updated patch, leaving the bug open due the above issues. Thank you.

[Darin Adler]

Comment on attachment 19655 [details]
New version of patch

Clearing the review flag since this patch was landed.

[Martin Robinson]

WebKit1GTK+ has been removed.