Closed Bug 583179 Opened 10 years ago Closed 10 years ago

Use MeegoTouch specific MApplication when MeegoTouch toolkit is enabled

Categories

(Toolkit :: Startup and Profile System, defect)

All
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: jap, Unassigned)

References

Details

Attachments

(1 file, 7 obsolete files)

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
Attachment #461468 - Flags: review?(doug.turner)
Blocks: 583135
Depends on: 583039, 582371
Blocks: 583148
Status: UNCONFIRMED → NEW
Ever confirmed: true
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!
Attachment #461468 - Flags: review?(doug.turner) → review-
(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.
Attachment #461468 - Attachment is obsolete: true
Attachment #461692 - Flags: review?
Attachment #461692 - Flags: review? → review?(doug.turner)
Attached patch Updated patch updated #define (obsolete) — Splinter Review
Updated patch according to updated patch from Bug 583039
Attachment #461692 - Attachment is obsolete: true
Attachment #462107 - Flags: review?(doug.turner)
Attachment #461692 - Flags: review?(doug.turner)
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.
Attachment #462107 - Flags: review?(doug.turner) → review-
(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
Attachment #462107 - Attachment is obsolete: true
Attachment #462172 - Flags: review?(doug.turner)
Simplify. Which is possible because we do not have the runtime getenv check anymore.
Attachment #462172 - Attachment is obsolete: true
Attachment #462179 - Flags: review?(doug.turner)
Attachment #462172 - Flags: review?(doug.turner)
Comment on attachment 462179 [details] [diff] [review]
Replace shared pointer with variable on stack

this needs a rebase or needs the dependent changes landed.
Attachment #462179 - Flags: review?(doug.turner) → review+
tracking-fennec: --- → ?
No longer depends on: 582371
Attachment #462179 - Attachment is obsolete: true
Attachment #462199 - Flags: review?(doug.turner)
Attached wrong file before.
Attachment #462199 - Attachment is obsolete: true
Attachment #462200 - Flags: review?(doug.turner)
Attachment #462199 - Flags: review?(doug.turner)
Attachment #462200 - Flags: review?(doug.turner) → review+
Keywords: checkin-needed
No longer blocks: 583135
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.
Attachment #462200 - Attachment is obsolete: true
Attachment #462429 - Flags: review?(doug.turner)
Attachment #462429 - Flags: review?(doug.turner) → review+
tracking-fennec: ? → 2.0+
Blocks: 584235
http://hg.mozilla.org/mozilla-central/rev/3e77780b8072
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Blocks: 584520
You need to log in before you can comment on or make changes to this bug.