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 615653 - nm-applet fails to build using -DGSEAL_ENABLE
nm-applet fails to build using -DGSEAL_ENABLE
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: nm-applet
git master
Other All
: Normal minor
: ---
Assigned To: Dan Williams
Dan Williams
: 625593 (view as bug list)
Depends on:
Blocks: 585391
 
 
Reported: 2010-04-13 14:05 UTC by Mirsal Ennaime
Modified: 2010-08-02 20:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix compilation using -DGSEAL_ENABLE (11.42 KB, patch)
2010-04-13 14:07 UTC, Mirsal Ennaime
none Details | Review
Fix compilation using -DGSEAL_ENABLE (revised patch with more consistant indentation) (11.25 KB, patch)
2010-04-13 14:53 UTC, Mirsal Ennaime
reviewed Details | Review
Revised patch avoiding unnecessary function call overhead (11.88 KB, patch)
2010-04-23 16:40 UTC, Mirsal Ennaime
needs-work Details | Review
Revised patch following Javier's review (11.86 KB, patch)
2010-06-06 20:18 UTC, Mirsal Ennaime
none Details | Review

Description Mirsal Ennaime 2010-04-13 14:05:25 UTC
see http://live.gnome.org/GnomeGoals/UseGseal
Comment 1 Mirsal Ennaime 2010-04-13 14:07:53 UTC
Created attachment 158606 [details] [review]
Fix compilation using -DGSEAL_ENABLE

This patch substitutes all direct access to Gtk+ object fields
with accessor functions in order to fulfill the UseGseal goal
as described at http://live.gnome.org/GnomeGoals/UseGseal
    
It also requires to bump the required Gtk+ version to 2.18
Comment 2 André Klapper 2010-04-13 14:14:42 UTC
The patch mixes spaces with tabs...
Comment 3 Mirsal Ennaime 2010-04-13 14:22:36 UTC
Ah, right, I'm not used to tabs for indentation, let me fix this.
Comment 4 Mirsal Ennaime 2010-04-13 14:53:28 UTC
Created attachment 158608 [details] [review]
Fix compilation using -DGSEAL_ENABLE (revised patch with more consistant indentation)

This patch substitutes all direct access to Gtk+ object fields
with accessor functions in order to fulfill the UseGseal goal
as described at http://live.gnome.org/GnomeGoals/UseGseal

It also requires to bump the required Gtk+ version to 2.18
Comment 5 André Klapper 2010-04-23 15:29:03 UTC
Review of attachment 158608 [details] [review]:

::: src/applet-device-gsm.c
@@ +734,2 @@
 	w = gtk_alignment_new (0.5, 0.5, 0, 1.0);
+	gtk_box_pack_start (GTK_BOX (gtk_dialog_get_content_area (dialog)), w, TRUE, TRUE, 0);

Looks like you call this four times in this function. Makes more sense to cache it in a variable first, to save some time.

::: src/applet-dialogs.c
@@ +747,2 @@
 	w = gtk_alignment_new (0.5, 0.5, 0, 1.0);
+	gtk_box_pack_start (GTK_BOX (gtk_dialog_get_content_area (dialog)),

Same here.
Comment 6 Mirsal Ennaime 2010-04-23 16:40:00 UTC
Created attachment 159449 [details] [review]
Revised patch avoiding unnecessary function call overhead

Thanks a lot for your review, André !
Comment 7 Javier Jardón (IRC: jjardon) 2010-06-06 16:06:56 UTC
Review of attachment 159449 [details] [review]:

Some comments (I'n not the module maintainer)

::: src/applet.c
@@ +2893,1 @@
+	if (applet->menu && (window = gtk_widget_get_window (applet->menu))) {

I think It's better

window = gtk_widget_get_window (applet->menu);
if (window)
  {
     screen = gdk_drawable_get_screen (window);

@@ +2910,3 @@
 
+	if (applet->context_menu && (window = gtk_widget_get_window (applet->context_menu))) {
+		screen = gdk_drawable_get_screen (window);

Same here
Comment 8 Mirsal Ennaime 2010-06-06 20:18:14 UTC
Created attachment 162884 [details] [review]
Revised patch following Javier's review

Thanks :)
Comment 9 Dan Williams 2010-08-02 19:47:25 UTC
*** Bug 625593 has been marked as a duplicate of this bug. ***
Comment 10 Dan Williams 2010-08-02 20:27:53 UTC
33bdfe50e95f5bc628f8cb55a53ef9fbe2eb4596 (master)
f7d629b6bd1ddcddcc449db83a999e3e65084fd2 (0.8.x)

are the bits that are compatible with GTK-2.14+.

The pieces that require GTK 2.18 are in separate commits:

0e85376c5e7512225023918461881cb13aea22f3 (master)
a55917cad258da5688d23db6fedfc2e24afdc819 (0.8.x)

I've updated the 0.8.x commit to work with GTK 2.14+ through GTK_CHECK_VERSION.  Thanks!