273877 - Stop using the argc/argv params passed to nsIAppShell::Create

Status: RESOLVED FIXED

(Core Graveyard :: GFX, defect)

Description

[Benjamin Smedberg]

In order to fix the command-line-handling APIs (see
http://wiki.mozilla.org/index.php/XUL:Command_Line_Handling), I need to solve
the dependencies of widget code on the command line. These dependencies also
cause weird problems for gecko embedders.

The solution I'm going to strongly recommend (and provide a patch for) requires
XUL applications to initialize gtk/qt/xlib in nsAppRunner.cpp; this means that
the argc/argv params to nsIAppShell::Create can be unused. We already
special-case gtk_init (for the splash screen I think), so this bug is mainly
about qt + xlib.

Comment 1

[Zack Rusin]

Attachment: Qt patch

This special cases Qt. Actually "special casing" is misleading because
QApplication should really be created in the main so this patch would allow me
to make the code in nsAppShell in Qt a _lot_ cleaner.

Comment 2

[Benjamin Smedberg]

Yeh, gimme a few hours to post my patch, willya? :-P

Comment 3

[Benjamin Smedberg]

Attachment: Remove command-line handling from the nsIAppShell equation.

timeless or somebody else will need to figure out a good way to handle the xlib
port

Comment 4

[Zack Rusin]

One more thing the:
+#if defined(MOZ_WIDGET_QT)
+#include <qapplication.h>
+#endif
part from xpfe/bootstrap/nsAppRunner.cpp has to go before  
#ifdef MOZ_X11
#include <X11/Xlib.h>
#endif /* MOZ_X11 */
Otherwise Qt gets mighty confused by X11 globals like None, CursorShape, FocusIn
or FocusOut and compilation will abort. You can also just #undef them before
including qapplication but it's just easier to change the order of includes

Comment 5

[Darin Fisher]

Comment on attachment 168338 [details] [diff] [review]
Remove command-line handling from the nsIAppShell equation.

looks good to me, r=darin

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 168338 [details] [diff] [review]
Remove command-line handling from the nsIAppShell equation.

      * @param aCmdLineService
      *        This is stored and passed to appshell components as they are
      *        initialized.

remove this part of the comment.

Comment 7

[Benjamin Smedberg]

fixed on trunk. Timeless, do you want a followup bug about xlib?

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: fix qt build

I'd request two reviews, but I can't, so I'll use the superreview flag for
that...

Comment 9

[Benjamin Smedberg]

Reopen this until QT is fixed.

Comment 10

[Benjamin Smedberg]

Comment on attachment 169304 [details] [diff] [review]
fix qt build

I don't know about the difference between TK_LIBS and MOZ_QT_LDFLAGS, but if
Zack says this is fine it's fine with me too. Note to the random world, we
probably need to update browser/app/Makefile.in etc with this change also.

Comment 11

[Zack Rusin]

Comment on attachment 169304 [details] [diff] [review]
fix qt build

Looks good to me. I find MOZ_QT_LDFLAGS a little clearer in places like this
though.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 169304 [details] [diff] [review]
fix qt build

ok, checked in with MOZ_QT_LDFLAGS.

maybe it would be better, though, to always link with $(TK_LIBS) here, instead
of the specialcasing for the different toolkits?


leaving bug open for the browser/app/Makefile.in thing bsmedberg mentioned
(which I won't fix)

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

This caused bug bug 281244.