Is this related to bug 107759?

Also applets need to be able to inherit transparent backgrounds.

Created attachment 20818 [details] [review]
Draft of a patch

Mark: I've written this. Is this ok or not? Is there anything else
that should be done?

Setting priority to HIGH in order to get someone review the patch :-)

I forgot this in tha patch:

Index: applet.c
===================================================================
RCS file: /cvs/gnome/gnome-panel/gnome-panel/applet.c,v
retrieving revision 1.223
diff -u -r1.223 applet.c
--- applet.c    19 Sep 2003 03:59:43 -0000      1.223
+++ applet.c    20 Oct 2003 16:05:50 -0000
@@ -1145,9 +1145,7 @@
  
        orientation_change (info, panel);
        size_change (info, panel);
-#ifdef FIXME_FOR_NEW_CONFIG
        back_change (info, panel);
-#endif
  
        if (type != PANEL_OBJECT_BONOBO)
                gtk_widget_grab_focus (applet);

Created attachment 21011 [details] [review]
Updated patch

The patch nearly works. I did a clock debug window (as in bug #107146)
and I see the background change correctly. The only problem is that it
seems the frame automatically returns to a coloured state (I can see
it with the good background during a tenth of a second). I can't find why.

Vincent: overall the patch looks pretty good - the only bit I really
don't like is making PanelBackground dependant on PanelWidget. I'd
like to maintain that split. How about something like:

typedef void (*PanelBackgroundChangedNotify) 
                           (PanelBackground *background,
                            gpointer         user_data);

void panel_background_init 
         (PanelBackground              *background,
          PanelBackgroundChangedNotify  notify_changed,
          gpointer                      user_data);


One other random nit:

  + We already have a size_allocate handler in panel-applet-frame.c
    so lets re-use that. Something like:

static void
panel_applet_frame_update_background_size 
                  (PanelAppletFrame *frame,
                   GtkAllocation    *old_allocation,
                   GtkAllocation    *new_allocation)
{
  if (old_allocation->x      == new_allocation->x &&
      old_allocation->y      == new_allocation->y &&
      old_allocation->width  == new_allocation->width &&
      old_allocation->height == new_allocation->height)
    return;

  if (background->type == PANEL_BACK_NONE ||
     (background->type == PANEL_BACK_COLOR && !background->has_alpha))
    return;

  panel_applet_frame_change_background (frame);
}

static void
panel_applet_frame_size_allocate (GtkWidget     *widget,
                                  GtkAllocation *allocation)
{
  ..
  GtkAllocation old_allocation;

  old_allocation = *widget->allocation;

  if (!frame->priv->has_handle) {
    GTK_WIDGET_CLASS (parent_class)->size_allocate (widget, 
                                                    allocation);
    panel_applet_frame_update_background_size (widget,
                                               &old_allocation,
                                               allocation);
    return;
  }

  ...

  panel_applet_frame_update_background_size (widget,
                                             &old_allocation,
                                             allocation);
}


Feel free to go ahead after you make those changes.

Its really good to see this finally getting close :-)

Created attachment 21024 [details] [review]
Updated patch.

The updated patch was modified after reading Mark's comments.

Mark: I'd appreciate if you could review the changes I made, because
I'm not sure I implemented the PanelBackgroundChangedNotify the way
you wanted it. If it's ok, I'll commit to HEAD. It's too much changes
for gnome-2-4, isn't it?

Please not that there's a change in panel-toplevel.c that should not
be in the patch.

Patch looks good - as you say, though, the changed_notify bit isn't
exactly how I'd do it:

 void
-panel_background_init (PanelBackground *background)
+panel_background_init (PanelBackground              *background,
+		       PanelBackgroundChangedNotify  notify_changed,
+		       GtkWidget                    *panel)

  + I'd make the last param "gpointer user_data"

+	PanelBackgroundChangedNotify notify_changed;
+	GtkWidget              *panel;

  + and also make this "gpointer user_data;"

+void
+panel_widget_background_changed (PanelBackground *background,
+				 gpointer        panel)
+{
+	g_return_if_fail (PANEL_IS_WIDGET (panel));
+	g_signal_emit (G_OBJECT (panel),
+		       panel_widget_signals [BACK_CHANGE_SIGNAL],
+		       0);

  + make this static and remove the prototype from the header
  + also make the second parameter a PanelWidget and cast the
    function when passing it to panel_background_init - see
    below

-	panel_background_init (&panel->background);
+	panel_background_init (&panel->background,
+			       &panel_widget_background_changed,
+			       widget);

  + no need to take the address of the function i.e.:

  panel_background_init (
    &panel-background,
    (PanelBackgroundChangedNotify) panel_widget_background_changed,
    panel);


Otherwise, looks good. Thanks again :-)

I committed the patch to HEAD after making the changes you mention.
Now, we need to understand why the applet don't keep the pixmap and
come back to a coloured background. Maybe we should open a new bug for
this...

I tried to figure out the last few issues to get this working for the
applets. Bug is filed as #128167

Closing this bug because Kevin's bug is much more useful now.