One persistent thorn in my side has been getting the default X Display. I've seen code for this for gtk/qt copied around about 5 times. I also ran into a case a few days ago when I wanted an "auto X pointer", on which XFree would be invoked when it went out of scope. That's enough to merit "X11Util" to me!

Comment on attachment 451499 [details] [diff] [review] Add X11Util.h for X11 toolkits Looks sensible to me, thanks. >+inline Display* >+XDisplay() >+{ >+ return DISPLAY(); >+} When I added GetXDisplay(), I initially called it XDisplay() but roc pointed out that it looked like an Xlib function. How about DefaultXDisplay()? I would move the DISPLAY stuff to this function, to avoid polluting the preprocessor's namespace. >+template<typename T> >+struct NS_STACK_CLASS AutoXFree NS_STACK_CLASS doesn't seem the right thing here. I think we really want NS_IS_NOT_SAFE_TO_COPY, but I don't know whether we have that.

Comment on attachment 451499 [details] [diff] [review] Add X11Util.h for X11 toolkits >diff --git a/gfx/public/X11Util.h b/gfx/public/X11Util.h >+#if defined(MOZ_WIDGET_GTK2) >+# include <gdk/gdkx.h> >+# define DISPLAY GDK_DISPLAY >+#elif defined(MOZ_WIDGET_QT) >+# include <QX11Info> >+# define DISPLAY QX11Info::display >+#else >+# error Unknown toolkit >+#endif I'm not in love with this #define of DISPLAY. I'd much rather separate implementations of XDisplay(), tbh.

Oright, meant to post the updated patch but got sidetracked.

Qt build busted http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1277169298.1277169408.27698.gz I think there is some conflict with Qt defines... like CursorShape

Comment on attachment 452939 [details] [diff] [review] Build fix for Qt If this problem pops up more, we should create a wrapper for QX11Info.h that fixes up this conflict automatically. It's going to be ugly copying and pasting this macro around everywhere QX11Info.h might be included before X.h.

Reported about Qt include problem: http://bugreports.qt.nokia.com/browse/QTBUG-11627