583179 - Use MeegoTouch specific MApplication when MeegoTouch toolkit is enabled

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Jan Arne Petersen]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.4) Gecko/20100622 Fedora/3.6.4-1.fc13 Firefox/3.6.4
Build Identifier: 

Use MeegoTouch specific MApplication when MeegoTouch toolkit is enabled

Reproducible: Always

Comment 1

[Jan Arne Petersen]

Attachment: Use MApplication and MApplicationService when MeegoTouch is enabled.

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 461468 [details] [diff] [review]
Use MApplication and MApplicationService when MeegoTouch is enabled.


>+ *
>+ * The Original Code is Mozilla Communicator client code.
>+ *
>+ * The Initial Developer of the Original Code is
>+ * Netscape Communications Corporation.
>+ * Portions created by the Initial Developer are Copyright (C) 2010
>+ * the Initial Developer. All Rights Reserved.
>+ *


wrong copyright text.  (netscape! woot! heh)


> #if defined(MOZ_WIDGET_QT)
> #if ( MOZ_PLATFORM_MAEMO == 6 )


what patch does this depend on?


>+#ifdef USE_MEEGOTOUCH
>+#include <mapplication.h>

what is USE_MEEGOTOUCH?


>+      MozMeegoAppService *appService = new MozMeegoAppService;
>+      app = QSharedPointer<QApplication>(new MApplication(gArgc, gArgv,appService));

nit: space between gArgv and appService



This looks fine (but I really don't know much about MeegoTouch.  Could you fix up the answer/fix above and post another patch?  Thanks for your help!

Comment 3

[Jan Arne Petersen]

Attachment: Updated patch. Fix license and some small stuff.

(In reply to comment #2)
> >+ * The Original Code is Mozilla Communicator client code.
> >+ *
> >+ * The Initial Developer of the Original Code is
> >+ * Netscape Communications Corporation.
> >+ * Portions created by the Initial Developer are Copyright (C) 2010
> >+ * the Initial Developer. All Rights Reserved.
> >+ *
> 
> wrong copyright text.  (netscape! woot! heh)

Fixed.
 
> > #if defined(MOZ_WIDGET_QT)
> > #if ( MOZ_PLATFORM_MAEMO == 6 )
> 
> what patch does this depend on?

That is defined in patch from bug 582371 - Enable VKB in Maemo 6 / Meego Qt Version.

> >+#ifdef USE_MEEGOTOUCH
> >+#include <mapplication.h>
> 
> what is USE_MEEGOTOUCH?

That is from patch for Bug 583039 - Add a build switch to enable MeegoTouch support. A guard to define compilation with Meego Touch widgets.

> >+      MozMeegoAppService *appService = new MozMeegoAppService;
> >+      app = QSharedPointer<QApplication>(new MApplication(gArgc, gArgv,appService));
> 
> nit: space between gArgv and appService

Fixed.

Comment 4

[Jan Arne Petersen]

Attachment: Updated patch updated #define

Updated patch according to updated patch from Bug 583039

Comment 5

[Doug Turner (:dougt)]

Comment on attachment 462107 [details] [diff] [review]
Updated patch updated #define

In bug 583039, we are going to make sure that the C #defines and the autoconf defines match.

Sry, going to need a new patch with MOZ_ENABLE_MEEGOTOUCH

Please drop the getenv of MOZ_NO_MEEGOTOUCH -- unless you are thinking about using this only in the parent process.  If that is the case, there are better ways to do that.

Comment 6

[Jan Arne Petersen]

Attachment: Updated patch updated #define dropped getenv

(In reply to comment #5)
> In bug 583039, we are going to make sure that the C #defines and the autoconf
> defines match.
> 
> Sry, going to need a new patch with MOZ_ENABLE_MEEGOTOUCH

Done

> Please drop the getenv of MOZ_NO_MEEGOTOUCH -- unless you are thinking about
> using this only in the parent process.  If that is the case, there are better
> ways to do that.

Done

Comment 7

[Jan Arne Petersen]

Attachment: Replace shared pointer with variable on stack

Simplify. Which is possible because we do not have the runtime getenv check anymore.

Comment 8

[Doug Turner (:dougt)]

Comment on attachment 462179 [details] [diff] [review]
Replace shared pointer with variable on stack

this needs a rebase or needs the dependent changes landed.

Comment 9

[Jan Arne Petersen]

Attachment: Rebased to not depend on other patches.

Comment 10

[Jan Arne Petersen]

Attachment: Rebase to not depend on other patches (updated).

Attached wrong file before.

Comment 11

[Jan Arne Petersen]

Attachment: Make sure meego touch is only used in parent process

I just realized that the environment variable was used to make sure that meego touch is not used in the child processes. That is still required. Updated patch check the process type.

Comment 12

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/3e77780b8072