1155943 - We already depend on glib higher than 2.2, drop checking for functions from older versions

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Bryan Quigley]

Attachment: drop_pre_glib2.2_support.patch

It's in toolkit/xre/nsAppRunner.cpp where it checks for the functions to exist before calling them.   We can just call them directly.

Comment 1

[Bryan Quigley]

Comment on attachment 8594287 [details] [diff] [review]
drop_pre_glib2.2_support.patch

Should be a relatively clean review if you agree with making Firefox/GTK hard depend on glib2.2+ (which I believe it likely does already)

Thanks!

Comment 2

[Karl Tomlinson (:karlt)]

Comment on attachment 8594287 [details] [diff] [review]
drop_pre_glib2.2_support.patch

Thanks.

>+  //these are both defined in glib2.2 and higher.

Please leave out this comment.
gtk_window_set_auto_startup_notification is not a GLib function, but is available from GTK+ 2.2.  This kind of comment could be helpful in the commit message, but is not necessary or helpful inline for understanding the code.

Comment 3

[Bryan Quigley]

Attachment: 0001-We-already-depend-on-glib2.2-and-gtk2.2-so-drop-the-.patch

Thanks for the review! I believe I made this patch correctly for check-in.

Comment 4

[Bryan Quigley]

Comment on attachment 8594765 [details] [diff] [review]
0001-We-already-depend-on-glib2.2-and-gtk2.2-so-drop-the-.patch

Review of attachment 8594765 [details] [diff] [review]:
-----------------------------------------------------------------

Try 2.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8594765 [details] [diff] [review]
0001-We-already-depend-on-glib2.2-and-gtk2.2-so-drop-the-.patch

In the future, the checkin-needed bug keyword is actually the preferred way of requesting checkin :)

Thanks for the patch!

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4abfadb3b726

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4abfadb3b726