After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 612485 - Does not compile with -DGSEAL_ENABLED
Does not compile with -DGSEAL_ENABLED
Status: RESOLVED FIXED
Product: gnome-utils
Classification: Deprecated
Component: baobab
trunk
Other Linux
: Urgent normal
: ---
Assigned To: Fabio Marzocca
gnome-utils Maintainers
Depends on:
Blocks: 585391
 
 
Reported: 2010-03-10 20:33 UTC by André Klapper
Modified: 2010-09-15 10:01 UTC
See Also:
GNOME target: 3.0
GNOME version: 2.29/2.30


Attachments
Fixes lots of GSEAL issues. Ports to GtkSpinner (53.59 KB, patch)
2010-05-07 23:18 UTC, Thomas Andersen
reviewed Details | Review
Remove unused macros (CENTER_X and CENTER_Y) (866 bytes, patch)
2010-05-08 15:07 UTC, Thomas Andersen
none Details | Review
[baobab] Port to GtkSpinner (30.52 KB, patch)
2010-06-03 21:42 UTC, Thomas Andersen
needs-work Details | Review
[gdict] Fix GSEAL issues (7.54 KB, patch)
2010-06-15 21:46 UTC, Thomas Andersen
none Details | Review
[screenshot] Fix GSEAL issues (1.33 KB, patch)
2010-06-15 21:49 UTC, Thomas Andersen
reviewed Details | Review
[gdict] Fix GSEAL issues (7.78 KB, patch)
2010-06-15 21:59 UTC, Thomas Andersen
reviewed Details | Review
[gdict] Fix GSEAL issues (8.12 KB, patch)
2010-06-22 19:35 UTC, Thomas Andersen
committed Details | Review
[screenshot] Fix GSEAL issues (1.49 KB, patch)
2010-06-22 19:53 UTC, Thomas Andersen
committed Details | Review
[baobab] Fix GSEAL issues (15.92 KB, patch)
2010-06-22 20:35 UTC, Thomas Andersen
needs-work Details | Review
[baobab] Fix GSEAL issues (16.08 KB, patch)
2010-06-22 23:13 UTC, Thomas Andersen
committed Details | Review
[baobab] Port to GtkSpinner v2 (30.12 KB, patch)
2010-07-14 22:49 UTC, Thomas Andersen
needs-work Details | Review
[screenshot] Fix GSEAL issues (4.23 KB, patch)
2010-07-16 05:46 UTC, Thomas Andersen
reviewed Details | Review

Description André Klapper 2010-03-10 20:33:31 UTC
This module does not build with -DGSEAL_ENABLED.
See http://live.gnome.org/GnomeGoals/UseGseal .

Note that maybe this report cannot be fixed yet, as GTK+ still misses some accessor functions (see bug 588389, bug 597610) needed for sealing.
Also see http://live.gnome.org/GTK%2B/3.0/PendingSealings for current status.

The jhbuild output posted here of course only lists the very first error when trying to compile.

baobab.c: In function ‘baobab_set_busy’:
baobab.c:112: error: ‘GtkWidget’ has no member named ‘window’
baobab.c:113: error: ‘GtkWidget’ has no member named ‘window’
baobab.c: In function ‘drag_data_received_handl’:
baobab.c:950: error: ‘GtkSelectionData’ has no member named ‘length’
baobab.c:953: error: ‘GtkSelectionData’ has no member named ‘data’
make[3]: *** [baobab-baobab.o] Error 1
make[2]: *** [all-recursive] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
Comment 1 Thomas Andersen 2010-05-07 23:18:54 UTC
Created attachment 160549 [details] [review]
Fixes lots of GSEAL issues. Ports to GtkSpinner

This fixes baobab + some issues in the other apps. Still more work to be done.

gdict-aligned-window.c:108 has:
window->type = GTK_WINDOW_TOPLEVEL;

I don't think there are any accessors for that. On the other hand I am not sure that is something that should be set like that...
Comment 2 Thomas Andersen 2010-05-08 15:07:05 UTC
Created attachment 160579 [details] [review]
Remove unused macros (CENTER_X and CENTER_Y)

Forgot to remove these in the former patch which makes them unused.
Comment 3 Emmanuele Bassi (:ebassi) 2010-05-10 15:22:11 UTC
Review of attachment 160549 [details] [review]:

the patch looks good, for the gnome-screenshot and gnome-dictionary bits.

could you please split the patch into separate commits, one for each sub-component? it would speed up review/commit.
Comment 4 Emmanuele Bassi (:ebassi) 2010-05-10 15:23:39 UTC
(In reply to comment #1)

> gdict-aligned-window.c:108 has:
> window->type = GTK_WINDOW_TOPLEVEL;
> 
> I don't think there are any accessors for that. On the other hand I am not sure
> that is something that should be set like that...

no, it shouldn't be necessary and can be safely removed - or at least replaced by a constructor property.
Comment 5 André Klapper 2010-05-28 09:43:38 UTC
Thomas, do you have some time to split up the patch?
Comment 6 Thomas Andersen 2010-06-03 21:42:00 UTC
Created attachment 162697 [details] [review]
[baobab] Port to GtkSpinner

Okay, I'll see if I can get it all split up before the release on monday.
Comment 7 Paolo Borelli 2010-06-03 22:04:23 UTC
Review of attachment 162697 [details] [review]:

::: baobab/src/baobab.c
@@ +90,3 @@
 		cursor = busy_cursor;
 
+		gtk_widget_show ( GTK_WIDGET (baobab.spinner));

remove extra space

@@ +100,3 @@
 	}
 	else {
+		gtk_widget_hide ( GTK_WIDGET (baobab.spinner));

ditto

@@ +574,3 @@
 	}
 
+	gtk_widget_set_size_request (GTK_WIDGET (spinner), size, size);

I have not tested, but I do not think this is needed or correct: size_request is different from setting size. Beside GtkSpinner should be able to figure out the size

::: baobab/src/baobab.h
@@ +70,3 @@
 	ContextMenu *chart_menu;
 	GtkWidget *toolbar;
+	GtkSpinner *spinner;

leave it defined as a widget
Comment 8 Thomas Andersen 2010-06-06 21:49:02 UTC
I agree that gtk_widget_set_size_request should not be used like that. However, the spinner is simply not shown if the call is removed.
Comment 9 André Klapper 2010-06-10 10:59:25 UTC
Hmm, so how to process?
As release-team soon plans to ship gtk 2.90 for 2.31.x this becomes urgent.
Comment 10 Thomas Andersen 2010-06-15 21:46:04 UTC
Created attachment 163739 [details] [review]
[gdict] Fix GSEAL issues
Comment 11 Thomas Andersen 2010-06-15 21:49:46 UTC
Created attachment 163740 [details] [review]
[screenshot] Fix GSEAL issues
Comment 12 Thomas Andersen 2010-06-15 21:59:52 UTC
Created attachment 163741 [details] [review]
[gdict] Fix GSEAL issues

v2. Forgot to remove the GTK_WINDOW_TOPLEVEL line.
Comment 13 Javier Jardón (IRC: jjardon) 2010-06-20 02:07:00 UTC
Review of attachment 163741 [details] [review]:

looks good, some minor comments

::: gnome-dictionary/src/gdict-aligned-window.c
@@ +191,3 @@
   			 &entry_x,
   			 &entry_y);
+  gdk_window_get_geometry (gtk_widget_get_window (align_widget),

you can store the GdkWindow variable and rehuse it.
GdkWindow *window = gtk_widget_get_window (align_widget);
Comment 14 Javier Jardón (IRC: jjardon) 2010-06-20 02:08:00 UTC
Review of attachment 163740 [details] [review]:

looks good, some minor comments:

::: gnome-screenshot/gnome-screenshot.c
@@ +517,3 @@
   main_vbox = gtk_vbox_new (FALSE, 18);
   gtk_container_set_border_width (GTK_CONTAINER (main_vbox), 5);
+  gtk_box_pack_start (GTK_BOX (gtk_dialog_get_content_area (GTK_DIALOG (retval))),

you can rehuse the content_area variable here
Comment 15 Thomas Andersen 2010-06-22 19:35:03 UTC
Created attachment 164339 [details] [review]
[gdict] Fix GSEAL issues

Updated patch. Thanks Javier
Comment 16 Thomas Andersen 2010-06-22 19:53:02 UTC
Created attachment 164342 [details] [review]
[screenshot] Fix GSEAL issues

screenshot patch updated
Comment 17 Emmanuele Bassi (:ebassi) 2010-06-22 20:01:25 UTC
Review of attachment 164339 [details] [review]:

looks good. please, commit to master.
Comment 18 Emmanuele Bassi (:ebassi) 2010-06-22 20:01:57 UTC
Review of attachment 164342 [details] [review]:

looks good. please, commit to master.
Comment 19 Thomas Andersen 2010-06-22 20:35:27 UTC
Created attachment 164344 [details] [review]
[baobab] Fix GSEAL issues

Looks like I forgot to split out the non-spinner fixes for baobab
Comment 20 Paolo Borelli 2010-06-22 20:47:59 UTC
Review of attachment 164344 [details] [review]:

::: baobab/src/baobab-cell-renderer-progress.c
@@ +47,3 @@
   cellprogress->priv->perc = 0;
 
+  gtk_cell_renderer_set_padding (GTK_CELL_RENDERER(cellprogress), 4, 4);

space before (

::: baobab/src/baobab-chart.c
@@ +306,2 @@
   chart = BAOBAB_CHART (widget);
+   gtk_widget_set_realized (widget, TRUE);

indentation seems off here

@@ +321,3 @@
   attributes_mask = GDK_WA_X | GDK_WA_Y | GDK_WA_VISUAL | GDK_WA_COLORMAP;
 
+  gtk_widget_set_window (widget, gdk_window_new (gtk_widget_get_parent_window (widget),

use local vars: all the nested calls are too messy

Also do not call get_window repeatedely

@@ +328,3 @@
+  gtk_widget_set_style (widget, gtk_style_attach (gtk_widget_get_style (widget),
+                                                  gtk_widget_get_window (widget)));
+  gtk_style_set_background (gtk_widget_get_style (widget),

same for get_style

::: baobab/src/baobab.c
@@ +111,3 @@
 	/* change the cursor */
+	if (gtk_widget_get_window (baobab.window)) {
+		gdk_window_set_cursor (gtk_widget_get_window (baobab.window), cursor);

do not get_window twice, store in a local var
Comment 21 Thomas Andersen 2010-06-22 23:13:36 UTC
Created attachment 164364 [details] [review]
[baobab] Fix GSEAL issues

Patch updated. Thanks for the review Paolo.
Comment 22 Cosimo Cecchi 2010-06-29 10:57:10 UTC
Review of attachment 164364 [details] [review]:

Hi, thanks for the updated patch; inlined some more comments.

::: baobab/src/baobab-cell-renderer-progress.c
@@ +47,3 @@
   cellprogress->priv->perc = 0;
 
+  gtk_cell_renderer_set_padding (GTK_CELL_RENDERER( cellprogress), 4, 4);

Wrong spacing around GTK_CELL_RENDERER.

::: baobab/src/baobab-chart.c
@@ +331,2 @@
+  style = gtk_style_attach (gtk_widget_get_style (widget), window);
+  gtk_widget_set_style (widget, style);

You should use gtk_widget_style_attach() here.
Comment 23 André Klapper 2010-07-02 17:22:54 UTC
Cosimo: Was the last comment an "Okay to commit with fixing the two issues listed"?
Comment 24 Cosimo Cecchi 2010-07-02 17:29:27 UTC
Yes, let's move this forward.
Comment 25 Javier Jardón (IRC: jjardon) 2010-07-03 01:39:51 UTC
Comment on attachment 164364 [details] [review]
[baobab] Fix GSEAL issues

committed with the cosimoc comments

commit 2803defc99a265f994823f19ea9b2da48e0e5100
Comment 26 Cosimo Cecchi 2010-07-03 10:43:12 UTC
So, the last thing missing for closing this bug now is to update the GtkSpinner port patch of baobab according to Paolo's review in comment #7.
Comment 27 Thomas Andersen 2010-07-14 22:49:26 UTC
Created attachment 165926 [details] [review]
[baobab] Port to GtkSpinner v2

Looks like there is still more GSEAL work to be done. Will look at it soon.

screenshot-dialog.c: In function ‘on_preview_expose_event’:
screenshot-dialog.c:87: error: ‘GtkWidget’ has no member named ‘style’
screenshot-dialog.c:103: error: ‘GtkWidget’ has no member named ‘window’
screenshot-dialog.c:104: error: ‘GtkWidget’ has no member named ‘style’
screenshot-dialog.c: In function ‘screenshot_dialog_set_busy’:
screenshot-dialog.c:394: error: ‘GtkWidget’ has no member named ‘window’
screenshot-dialog.c:399: error: ‘GtkWidget’ has no member named ‘window
Comment 28 Thomas Andersen 2010-07-14 22:57:15 UTC
Review of attachment 165926 [details] [review]:

::: baobab/src/baobab.c
@@ +613,3 @@
 
+	baobab.spinner = gtk_spinner_new ();
+	gtk_widget_set_size_request (baobab.spinner, 1, 1);

This is probably not right either but without the call the widget does not appear.
Comment 29 Thomas Andersen 2010-07-16 05:46:12 UTC
Created attachment 166007 [details] [review]
[screenshot] Fix GSEAL issues

Fixes for gnome-screenshot. There is still work to be done for at least gsearchtool. Sorry. I must have stopped halfway with the first patch :(
Comment 30 Thomas Andersen 2010-07-16 05:48:42 UTC
Review of attachment 166007 [details] [review]:

::: gnome-screenshot/screenshot-xfer.c
@@ +127,2 @@
   widget = gtk_progress_bar_new ();
+  gtk_box_pack_start (GTK_BOX (content_area), widget, FALSE, 0, 0);

Not sure about this one as ->label->parent != content_area. Untested.
Comment 31 Cosimo Cecchi 2010-07-23 12:34:08 UTC
Review of attachment 166007 [details] [review]:

I inlined a comment below. Other than this, looks fine.

::: gnome-screenshot/screenshot-xfer.c
@@ +127,2 @@
   widget = gtk_progress_bar_new ();
+  gtk_box_pack_start (GTK_BOX (content_area), widget, FALSE, 0, 0);

Use gtk_message_dialog_get_message_area () here.
Comment 32 Paolo Borelli 2010-07-23 12:57:49 UTC
Review of attachment 165926 [details] [review]:

Sorry for the delay.

I tested the updated patch, but the spinner is not sized correcly when the toolbar is set to both.

After thinking about it a bit, I think the workaround proposed in your previous patch where the size of the toolbar is looked up from the conf is the lesser evil
Comment 33 Paolo Borelli 2010-07-23 13:01:41 UTC
Review of attachment 165926 [details] [review]:

also, when doing the size request, only set the height
Comment 34 Paolo Borelli 2010-07-24 09:35:17 UTC
I fixed up the spinner patch and pushed it (btw, only setting the height as suggested by hadess did not work)
Comment 35 Javier Jardón (IRC: jjardon) 2010-09-01 15:16:29 UTC
Seems that this is fixed now, Can we close this bug now?
Comment 36 Cosimo Cecchi 2010-09-15 10:01:45 UTC
Seems so.