Closed Bug 78498 Opened 23 years ago Closed 23 years ago

PPEmbed needs to be Carbonized

Categories

(Core Graveyard :: Embedding: APIs, enhancement)

Product:
Core Graveyard
Component:
Embedding: APIs
Platform:
PowerPC
macOS
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.4

People

(Reporter: ccarlen, Assigned: ccarlen)

Details

Attachments

(2 files)

patch to PPEmbed
61.21 KB, patch
Details | Diff | Splinter Review
patch to build scripts
2.61 KB, patch
Details | Diff | Splinter Review 
I have a Carbonized version of PPEmbed which uses PowerPlant 2.1 in my tree.
Will check it in as time permits.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.2
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.

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
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.

cc sdagley

> 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.
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.

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.

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.

Here's another good reason why we don't want to be too eager to distribute Carbon 
builds: bug 71718.

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
*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.

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.

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=
Pink, can you r=? Scott, since Simon is away, that leaves you for sr=.
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.

> 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

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=
-> 0.9.4 due to lack of sr=.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
sr=sfraser

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
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: need sr=
QA Contact: mdunn → depstein
verified against PPEmbed 2002051608 trunk (MacEmbed). works fine on OSX.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: