PPEmbed needs to be Carbonized

VERIFIED FIXED in mozilla0.9.4

Status

()

Core
Embedding: APIs
--
enhancement
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: Conrad Carlen (not reading bugmail), Assigned: Conrad Carlen (not reading bugmail))

Tracking

Trunk
mozilla0.9.4
PowerPC
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

17 years ago
I have a Carbonized version of PPEmbed which uses PowerPlant 2.1 in my tree.
Will check it in as time permits.
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.2
(Assignee)

Comment 1

17 years ago
Simon, I'd like to start checking this stuff in since it's just sitting in my
tree. As the first step, and one for which I'd like you and Pink's r=/sr=, I'd
like to check in a revamped lib/mac/powerplant/PowerPlant.mcp. What it does is this:
(1) Builds powerplant as a separate static lib. All the powerplant src files can
then be removed from the embedding sample.
(2) It has 4 targets: PowerPlant[Carbon][Debug]. The Carbon targets use
PowerPlant 2.1 from Fizilla Tools. The non-Carbon targets use PowerPlant from
the standard build tools. In order for PowerPlant 2.1 to compile with CW 5.3 and
the headers on Fizilla Tools, I had to alter 3 PowerPlant files. These are in a
dir inside lib/mac/powerplant and get used by access path trickery. I did this
in order to not touch any PowerPlant files inside the CW folder.
(3) PowerPlant.mcp exports to dist/mac/powerplant a static lib and a
pre-compiled header. The embedding sample will use the same pre-compiled header
and its own pre-compiled header can go away.

Forgive the long-winded description of this stuff. The joy of just being able
post diffs to a makefile is not ours.

Comment 2

17 years ago
This worries me, because users of the Carbonized PPEmbed will suddenly start 
running into Carbon-on-OS-9 bugs that we are otherwise brushing under the carpet 
(since we've stated that we are not supporting this configuration). See, for 
example, bug 68673.

Are we promising to embedders that Mozilla under Carbon on 9 is a supported 
configuration? If so, then we really need to bring Carbon/9 to the fore for 
Mozilla as well (otherwise you'll be in a world of hurt).
i'm not sure why carbonizing the PPEmbed sample necessarily says we support it to 
run on os9. Remember, people can build carbon apps that only run on X using PB. 
Carbon isn't just for super-binaries anymore.

Conrad, what are these PowerPlant files you had to change? Do you mean you want 
to check them into the tree in lib/mac/powerplant? That would probably be bad, 
and make the lawyers very unhappy.
Summary: PPEmbed needs to be Carbobized → PPEmbed needs to be Carbonized
(Assignee)

Comment 4

17 years ago
Simon - as far as running a Carbon embedding app on OS 9, we would have to find
out how important this is to embeddors on the Mac. I imagine it would be.

Mike - the changed PowerPlant files have to do with our build tools. PP 2.1 uses
an aspect of template support (outlined template member functions) which the 5.3
compiler does not support. Since, as you point out, this might make lawyers
unhappy, this may have to wait until we update our build system, and we're using
a CW with Carbonized PowerPlant included. Sigh.

Comment 5

17 years ago
cc sdagley
(Assignee)

Comment 6

17 years ago
> This worries me, because users of the Carbonized PPEmbed will suddenly start 
running into Carbon-on-OS-9

Why is this more of a concern with the embedding sample than with mozilla? As I
said, there are separate Carbon and classic targets in the embedding sample. I
didn't say that I had configured the embedding sample to be Carbon-only for OS 9
and X.

Comment 7

17 years ago
As an embedder, I'm very anxious to get my hands on the Carbonized version of PPEmbed, so I can get my own application up and running on Mac OS X.

I agree with Conrad that the PPEmbed shouldn't be any more of a concern than Fizilla itself. If we are worried about someone using the Carbonized version under Mac OS 9, then how about "hacking" in a gestalt check for the OS version in the official distribution version at initialization that will throw up a warning alert if the OS version is prior to 10.0.0 telling the user not to submit bug reports on this build.

Just my $0.02.

Comment 8

17 years ago
This is more of a concern with PPEmbed than mozilla because we have less control 
over who uses it. If we provide a Carbon-compliant embedding build, then we 
_have_ to commit to making it work under Carbon on 9. Up until now, we've punted 
on this, saying that Mozilla will only be qualified on 9/Classic, and X/Carbon.

Comment 9

17 years ago
Simon, as an embedder, I disagree. I'm more than willing to live with the
limitation that the Carbon build is only "supported" by Mozilla (the team) 
when run under Mac OS X. 

I recommend you put in a alert in the release distribution, as I noted above.
Better yet, if we're that worried about it, let's put in an ExitToShell call
after the alert and force the program to terminate if it's running on OS 9.

An embedder would have to build his own distribution in order to hack out 
the dialog and ExitToShell in order to run on Mac OS 9. If they do that, then they've obviously read the alert and deserve any errors/bombs they get.

Let's not slow down people who want to adopt Mac OS X because they "might" try
to use it on Mac OS 9.

Comment 10

17 years ago
Here's another good reason why we don't want to be too eager to distribute Carbon 
builds: bug 71718.
(Assignee)

Comment 11

17 years ago
I don't see why bugs like 71718 should block having PPEmbed in a buildable state
under Carbon. Are we going to say that every OSX bug in mozilla has to be fixed
before embeddors can get started with embedding in PowerPlant 2.1? This holds
them back unnescesarily and doesn't give *them* a chance to assess the
Carbon-only bugs for themselves and possibly do something about them.
Target Milestone: mozilla0.9.2 → mozilla0.9.3

Comment 12

17 years ago
*shrug* I just don't want carbon embedders to assume that everything is going to 
work as well as it does in the classic build.

Comment 13

17 years ago
I sympathize Simon's concern over setting some precident which can be very
dangerous. However, IMHO, there's a schlew of configurations across mozilla at
large that we don't "officially" support, yet there's code checked in to try and
use mozilla in those twisted OS configs.

One thing we've had to do in the embedding group is draw a vivid line between
supported code and non-supported code; *and* stand by those decisions. We could
just as easily draw a line here. The "Platform Support" section at the top of
http://www.mozilla.org/projects/embedding/ should be ellaborated on regardless
of the outcome of PPEmbed, but that section could contain the mozilla core
supported platforms very easily (can someone fill in the mac part for me and
I'll post it to gila?), and carbon os9 might not be one that list. Everything
else is a "use at your own risk, and don't be upset when/if your bugs get
futured/invalidated" option.

I'm w/ Conrad here though. We can easily explain that carbon/0S9 isn't
supported, use at your own risk, while providing the target. 

It sounds like he's whacked the project file w/ other benefits as well (1, and 3
seem independent and of benefit regardless).

my 2 cents. we shouldn't under-estimate our newfound (still maturing :-))
ability to hold true to our decisions and enforce them, while still supporting
forward/tangential progress.
(Assignee)

Comment 14

17 years ago
Created attachment 42097 [details] [diff] [review]
patch to PPEmbed
(Assignee)

Comment 15

17 years ago
Created attachment 42098 [details] [diff] [review]
patch to build scripts
(Assignee)

Comment 16

17 years ago
In addition to the changes in order to Carbonize the embedding sample, this
patch incorporates a bunch of other stuff that I've done here and there and not
checked in.
1. Browser windows are made differently in order to make them out of 'ppob' view
components which correspond to chrome flags.
2. View Source (from context menu too)
3. WebBrowserPersist to allow saving URIs
4. A facelift which uses 24-bit icons for buttons instead of text buttons.

Looking for r=/sr=
(Assignee)

Comment 17

17 years ago
Pink, can you r=? Scott, since Simon is away, that leaves you for sr=.
(Assignee)

Updated

17 years ago
Whiteboard: need r=/sr=
there's a lot there, so i didn't look too closely FYI. two comments, but not 
worth holding up for:

1) the readme says to use version 2.2.1 of powerplant. where do i get that? is 
that what ships with pro6? or with an update to pro6? you might want to be more 
specific

2) if the saveFile code, you use UConditionalDialogs instead of 
UNavServicesDialogs when not under carbon. In non-embedding, we always use 
navServices even if not running under carbon. was there a reason for this? 
something internal to powerplant?

with the build changes, why do you bother checking a different flag for 
embedding_test_carbon? will we ever want to build the non-carbon embedding sample 
when in teh fizzilla build environment? I didn't even think that was possible. 
can you clarify why this is necessary?

otherwise, r=pink.
(Assignee)

Comment 19

17 years ago
> the readme says to use version 2.2.1 of powerplant.

I used what was on "Fizzilla Tools" Dagley would know the details of what's
there and where it came from. Steve?

> if the saveFile code, you use UConditionalDialogs instead of UNavServicesDialogs

UConditionalDialogs is preferable because, by default, it will use NavServices
if available. That should be the case on any system on which mozilla can run.
The better reason is that, with UConditionalDialogs, the use of NavServices can
be globally turned off. If somebody is using this code in a PowerPlant app which
makes use of this (in some apps it's a pref), it should allow that. It sucks
that Metrowerks made UConditionalDialogs unavailable under Carbon. The condition
is pretty obvious: it's always NavServices. It just makes for annoying
conditional compilation.

> with the build changes, why do you bother checking a different flag for 
embedding_test_carbon?

Because I want the building of this to default to "on" under Classic and "off"
under Carbon. If somebody wants to go out of their way and get PowerPlant 2.1.1
and modify it so they can compile with 5.3, they can and then define the build
flag which causes it to build. The script checks for the right combination of
these and won't build the Carbon embedding app in a Classic build or vice-versa.
If there was just one "embedding_test" build flag, I'd have to change the
scripts to check for carbon (which isn't known until the user build prefs are
read) and if carbon and if "embedding_test" is not set in the user build prefs,
turn it off. Yikes.
sounds muy bueno. re-asserting r=pink
(Assignee)

Comment 21

17 years ago
Thanks.

Steve, could you tell us what version of PowerPlant is on "Fizzilla Tools" and
where it came from? I would like to add that to the README.
Scott, can you sr?
Whiteboard: need r=/sr= → need sr=
(Assignee)

Comment 22

17 years ago
-> 0.9.4 due to lack of sr=.
Target Milestone: mozilla0.9.3 → mozilla0.9.4

Comment 23

17 years ago
sr=sfraser
(Assignee)

Comment 24

17 years ago
Checked in. ReadMe-Carbon for what you need to build it.
It's off by default. To build it, put:
options embedding_test_carbon 1
in your build prefs.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Whiteboard: need sr=

Updated

17 years ago
QA Contact: mdunn → depstein

Comment 25

16 years ago
verified against PPEmbed 2002051608 trunk (MacEmbed). works fine on OSX.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.