GNOME Bugzilla – Bug 612485
Does not compile with -DGSEAL_ENABLED
Last modified: 2010-09-15 10:01:45 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
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...
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.
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.
(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.
Thomas, do you have some time to split up the patch?
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.
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
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.
Hmm, so how to process? As release-team soon plans to ship gtk 2.90 for 2.31.x this becomes urgent.
Created attachment 163739 [details] [review] [gdict] Fix GSEAL issues
Created attachment 163740 [details] [review] [screenshot] Fix GSEAL issues
Created attachment 163741 [details] [review] [gdict] Fix GSEAL issues v2. Forgot to remove the GTK_WINDOW_TOPLEVEL line.
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);
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
Created attachment 164339 [details] [review] [gdict] Fix GSEAL issues Updated patch. Thanks Javier
Created attachment 164342 [details] [review] [screenshot] Fix GSEAL issues screenshot patch updated
Review of attachment 164339 [details] [review]: looks good. please, commit to master.
Review of attachment 164342 [details] [review]: looks good. please, commit to master.
Created attachment 164344 [details] [review] [baobab] Fix GSEAL issues Looks like I forgot to split out the non-spinner fixes for baobab
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
Created attachment 164364 [details] [review] [baobab] Fix GSEAL issues Patch updated. Thanks for the review Paolo.
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.
Cosimo: Was the last comment an "Okay to commit with fixing the two issues listed"?
Yes, let's move this forward.
Comment on attachment 164364 [details] [review] [baobab] Fix GSEAL issues committed with the cosimoc comments commit 2803defc99a265f994823f19ea9b2da48e0e5100
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.
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
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.
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 :(
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.
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.
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
Review of attachment 165926 [details] [review]: also, when doing the size request, only set the height
I fixed up the spinner patch and pushed it (btw, only setting the height as suggested by hadess did not work)
Seems that this is fixed now, Can we close this bug now?
Seems so.