Use MeegoTouch specific MApplication when MeegoTouch toolkit is enabled

RESOLVED FIXED

Status

()

Toolkit
Startup and Profile System
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Jan Arne Petersen, Unassigned)

Tracking

unspecified
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(fennec2.0+)

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

8 years ago
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
(Reporter)

Comment 1

8 years ago
Created attachment 461468 [details] [diff] [review]
Use MApplication and MApplicationService when MeegoTouch is enabled.
Attachment #461468 - Flags: review?(doug.turner)
(Reporter)

Updated

8 years ago
Blocks: 583135
Depends on: 583039, 582371

Updated

8 years ago
Blocks: 583148

Updated

8 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

8 years ago
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-
(Reporter)

Comment 3

8 years ago
Created attachment 461692 [details] [diff] [review]
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.
Attachment #461468 - Attachment is obsolete: true
Attachment #461692 - Flags: review?
(Reporter)

Updated

8 years ago
Attachment #461692 - Flags: review? → review?(doug.turner)
(Reporter)

Comment 4

8 years ago
Created attachment 462107 [details] [diff] [review]
Updated patch updated #define

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 5

8 years ago
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-
(Reporter)

Comment 6

8 years ago
Created attachment 462172 [details] [diff] [review]
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
Attachment #462107 - Attachment is obsolete: true
Attachment #462172 - Flags: review?(doug.turner)
(Reporter)

Comment 7

8 years ago
Created attachment 462179 [details] [diff] [review]
Replace shared pointer with variable on stack

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 8

8 years ago
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+

Updated

8 years ago
tracking-fennec: --- → ?
(Reporter)

Updated

8 years ago
No longer depends on: 582371
(Reporter)

Comment 9

8 years ago
Created attachment 462199 [details] [diff] [review]
Rebased to not depend on other patches.
Attachment #462179 - Attachment is obsolete: true
Attachment #462199 - Flags: review?(doug.turner)
(Reporter)

Comment 10

8 years ago
Created attachment 462200 [details] [diff] [review]
Rebase to not depend on other patches (updated).

Attached wrong file before.
Attachment #462199 - Attachment is obsolete: true
Attachment #462200 - Flags: review?(doug.turner)
Attachment #462199 - Flags: review?(doug.turner)

Updated

8 years ago
Attachment #462200 - Flags: review?(doug.turner) → review+

Updated

8 years ago
Keywords: checkin-needed

Updated

8 years ago
No longer blocks: 583135
(Reporter)

Comment 11

8 years ago
Created attachment 462429 [details] [diff] [review]
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.
Attachment #462200 - Attachment is obsolete: true
Attachment #462429 - Flags: review?(doug.turner)
Keywords: checkin-needed

Updated

8 years ago
Attachment #462429 - Flags: review?(doug.turner) → review+

Updated

8 years ago
tracking-fennec: ? → 2.0+

Updated

8 years ago
Blocks: 584235
http://hg.mozilla.org/mozilla-central/rev/3e77780b8072
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Blocks: 584520
You need to log in before you can comment on or make changes to this bug.