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

RESOLVED FIXED in Firefox 40

Status

()

--
minor
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: firefox, Assigned: firefox)

Tracking

Trunk
mozilla40
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Created attachment 8594287 [details] [diff] [review]
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.
(Assignee)

Comment 1

3 years ago
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!
Attachment #8594287 - Flags: review?(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.
Attachment #8594287 - Flags: review?(karlt) → review+
(Assignee)

Comment 3

3 years ago
Created attachment 8594765 [details] [diff] [review]
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.
Attachment #8594765 - Flags: checkin?
(Assignee)

Comment 4

3 years ago
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.
Attachment #8594765 - Flags: checkin? → checkin?(karlt)
Attachment #8594287 - Attachment is obsolete: true
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!
Attachment #8594765 - Flags: checkin?(karlt)
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/4abfadb3b726
Assignee: nobody → gQuigs+bugs
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4abfadb3b726
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40

Updated

3 years ago
Depends on: 1157413
No longer blocks: 1157869
Depends on: 1157869

Updated

3 years ago
Depends on: 1158154
You need to log in before you can comment on or make changes to this bug.