Closed Bug 318331 Opened 15 years ago Closed 13 years ago

Default build does not support Xinerama - detect support at runtime

Categories

(Core :: Widget: Gtk, enhancement)

x86
Linux
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9alpha5

People

(Reporter: seth, Assigned: sylvain.pasche)

References

Details

Attachments

(1 file, 5 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

The default build of FF 1.5 does not support Xinerama (multi-head on X).  This means that dialogs and prompts (such as preferences) do not open in the center of one screen but overlap both (in a 2 head system).

Reproducible: Always

Steps to Reproduce:
1. Start Firefox
2. Open Edit -> Preferences

Actual Results:  
Window appears in center of workspace, overlapping on both screens

Expected Results:  
Window should appear in the center of the active screen.
Version: unspecified → 1.5 Branch
Xinerama can't be enabled by default, as it is possible that some distributions don't include that library:

See the thread: http://groups.google.com/group/mozilla.dev.builds/browse_frm/thread/ae8c08a4f338ccd4/04442d26536b4281#04442d26536b4281 
Status: UNCONFIRMED → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
I'll move this to the right component in a bit, and I'll explain why I'm reopening.
Status: RESOLVED → UNCONFIRMED
Resolution: WONTFIX → ---
Re: Sylvain's comment on the newsgroup thread he mentioned in comment #1.

We don't build with it on by default, but in theory it would be possible to do runtime detection. Just because we don't right now, doesn't mean we could never do that. Morphing into a Widget bug.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Component: OS Integration → Widget: Gtk
Ever confirmed: true
Product: Firefox → Core
QA Contact: os.integration → gtk
Summary: Default build does not support Xinerama → Default build does not support Xinerama - detect support at runtime
Version: 1.5.0.x Branch → Trunk
Attached patch v1 (obsolete) — Splinter Review
This is a first proof of concept. I'd be happy to know if such an approach has chances to be included.

I did not test this on a non Xinerama enabled system yet, as my Ubuntu 7.04 links automatically against Xinerama because of gtk or some other libs.

Remaining tasks:
- drop xinerama detection and defines from build system
- port the changes to gtk1
Assignee: nobody → sylvain.pasche
Status: NEW → ASSIGNED
This looks like the right way to go, yeah. Don't bother supporting GTK1, it's dead on trunk.
After some more surveying of distributions, I fear that some of my initial arguments were wrong. Apparently, they all match the requirements:

http://groups.google.com/group/mozilla.dev.builds/browse_thread/thread/ae8c08a4f338ccd4/3e0910693147f42e#3e0910693147f42e

That would mean there's little gain in loading it dynamically. I'll probably open another bug for automatic detection by autoconf, or tinderbox config change.
Attached patch v2 (obsolete) — Splinter Review
Attachment #260391 - Attachment is obsolete: true
After some discussions with bsmedberg, we're probably going to enable Xinerama by default instead. See bug 376658.

This bug may still be useful in the very improbable case a supported distribution does not ship with Xinerama any more in the future. I attached my last version for reference, but it's not meant to be used for now.
Assignee: sylvain.pasche → nobody
Status: ASSIGNED → NEW
Attached patch v2 (obsolete) — Splinter Review
real one
Attachment #260770 - Attachment is obsolete: true
Blocks: 377259
Attached patch v2.1 trunk updated (obsolete) — Splinter Review
Finally this may still make some sense, see bug 377259, comment 6
Assignee: nobody → sylvain.pasche
Attachment #260774 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #261701 - Flags: review?(roc)
Comment on attachment 261701 [details] [diff] [review]
v2.1 trunk updated

+    PRLibrary* xineramalib = PR_LoadLibrary("libXinerama.so.1");
+    if (xineramalib) {

You're leaking xineramalib, which I think is OK because there is really no other way to do this, but you should say so in a comment.
Attachment #261701 - Flags: superreview+
Attachment #261701 - Flags: review?(roc)
Attachment #261701 - Flags: review+
Attached patch v3 (obsolete) — Splinter Review
Added the comment about the leak (I also removed a small thing which shouldn't have been part of the patch).
Attachment #261701 - Attachment is obsolete: true
Whiteboard: [checkin needed]
Sorry for the delay checking this in - the patch no longer applies cleanly (looks like some trivial merge is needed, but I can't test Linux right now). Could you attach an updated patch so that I can check it in?
Target Milestone: --- → mozilla1.9alpha5
Attached patch v3.1Splinter Review
Updated to trunk
Attachment #261773 - Attachment is obsolete: true
mozilla/configure.in                            1.1804
mozilla/config/autoconf.mk.in                   3.424
mozilla/config/static-config.mk                 3.35
mozilla/config/system-headers                   3.14
mozilla/toolkit/library/Makefile.in             1.51
mozilla/widget/src/gtk2/Makefile.in             1.63
mozilla/widget/src/gtk2/nsScreenGtk.cpp         1.25
mozilla/widget/src/gtk2/nsScreenGtk.h           1.12
mozilla/widget/src/gtk2/nsScreenManagerGtk.cpp  1.15
Status: ASSIGNED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
You need to log in before you can comment on or make changes to this bug.