Closed Bug 18352 Opened 20 years ago Closed 18 years ago

[META] build should not require extensions/


(SeaMonkey :: Build Config, defect, P1, major)


(Not tracked)



(Reporter: cls, Assigned: netscape)



(Keywords: arch, embed, meta)


(7 files)

The point of the extensions/ directory was to put optional things there.  If you
attempt to build the browser with that directory missing, it will crash.  If
nsIWalletService.h & nsICookieService.h are moved into another directory (say
mozilla/include/), the browser builds fine without the extensions directory.

So, the question is...where do we move these header files?  They are needed to
provide the interface to Mozilla but the directories that they are currently in
are not needed.
I nominate netwerk/base/public or netwerk/protocol/http/public for
nsICookieService.idl.  The wallet interface should probably live somewhere like
layout/html/forms/public, but I'm not as sure about that.
Assignee: morse → brendan
Actually, this is the first I've heard that the browser should be able to build
with the extensions directory missing.  Is this really a requirement?

In any case, I'm deferring this to Brendan since he was the original advocate of
creating an extensions when we were looking for a home for wallet.
Assignee: brendan → morse
I can only repeat what cls wrote: "The point of the extensions/ directory was to
put optional things there."  Extension and optional have pretty clear and, in
most software, related meanings.  For example, all Emacs versions I know of can
be built and run without the many extensions that might be bundled as source and
even binary (.fasl, whatever) optional added parts (which is to say, as

This should be fixed, in particular it seems to me that wallet hookup from
layout code should use a public API purveyed from layout.  I'm not the person to
fix this, though.  Steve, now that I've restated the purpose of extensions, will
you fix it?  If not, please bounce to dp.

Closed: 20 years ago
Resolution: --- → DUPLICATE
This has nothing to do with layout -- the dependency with layout was
removed a long time ago.

I just investigated and the cause of this dependency is the fact that wallet and
cookie are initialized in the webshell.  So this is really a duplicate of bug

*** This bug has been marked as a duplicate of 14889 ***
Depends on: 14889
Even if the initialization of the cookie & wallet components is moved from
webshell to somewhere under necko (according to <a
href="">bug 17120</a>), the
header files will still need to be moved.
Closed: 20 years ago20 years ago
That's not the case.  The latest thinking on bug 14889 is the following (quoting
from a message from Brendan):

"have the app, at startup, add a tiny observer for the Cookie: header, but don't
link in or otherwise spend cycles on getting the Cookie service and initializing
it.  Do all that from the observer when it actually sees the first header.  The
observer that gets the Cookie service and calls into it can be a tiny part of
the browser app."

So this tiny observer is a built-in part of the browser and will not be in the
extensions file.  It then accesses the cookie dll through the xpcom interface.
So the header files for the cookie dll will remain in the extensions directory
and will not cause any problems.

Wallet will be handled in a similar manner.
Ok, I'm confused.  Isn't nsICookieService.h needed to define the cookie
interface for the tiny observer?  If so, isn't it (part of the main browser)
still going to need to look under extensions/ for that header?  Or do you plan
to redefine those interfaces in the tiny observer?
I guess all this still needs more thought.  So I'll keep this bug open and
distinct from 14889 just to make sure both issues get addressed.
Resolution: DUPLICATE → ---
Target Milestone: M13
Target Milestone: M13 → M14
Summary: Cannot build browser if extensions dirs are missing → [feature] Cannot build browser if extensions dirs are missing
Assignee: morse → dp
Reassigning to dp since this he can fix this at the same time that he fixes bug
Target Milestone: M14 → M17
I dont think we will fix this goodness before shipping Netscape 6

Keywords: helpwanted
Target Milestone: M17 → M20
it really depends on who you talk to, dp. I'll see if i can help at all to make 
extensions really *mean* extensions.
mass re-assign of all bugs where i was listed as the qa contact
QA Contact: cyeh → chofmann
C'mon, guys, this is just getting worse.  Now you can't build libimg without a
dependency on the cookie ``extension'', because it has a hard-coded dependency
on nsICookieService and such.  (Since if.cpp 3.46, March 19,,
no reviewer listed (!).)

(To say nothing of the fact that images have nothing to do with cookies, but
I'll file another bug about the interface-scope-creep issue later.)

It's one thing to say ``I don't have time to go back and fix the existing
brokenness'' but it's another entirely to just keep piling the brokenness on. 
What gives?
For the record, the way to fix this was moving cookie (not cookie manager, the 
UI) into necko. We have concensus on doing that with the necko group.
Are you going to move the non-cookie-related API nonsense for images in there as
well?  Seems like that stuff should really be in a separate interface, seeing as
how image loading and cookie disposition are pretty much unrelated.

Also, will the cookie policy stuff no longer be pluggable?  There is interest in
some circles WRT replacing the default Mozilla cookie policy with something more
flexible, or integrated with other pieces of software, so I'd be a little
saddened to hear that we were just going to pile this stuff into necko, rather
than doing the legwork originally implied by the extensions/ location.

(Also, will we still have to build wallet to get the UI for managing the cookie
policy?  That seems like another case of conflation that is better undone than
I dont know much about the image manager. It is a recent addition.

Anyway, eventhough we are going to move the cookie code into necko, it will be 
triggered by cookie notify using categories. So an external cookie module can 
implement its own cookie notify and replace the existing CID under the 
http-startup category and they will be in business. Moving it wont impose any 
new restrains. It will save us from one more dll.

ok ?
Embedding needs imagelib, but it certainly doesn't need cookies. Nominating for 
nsbeta3, and cc'ing valeski.
Keywords: helpwantedembed, nsbeta3
Summary: [feature] Cannot build browser if extensions dirs are missing → Cannot build browser if extensions dirs are missing
Only embedding, no help for seamonkey.
Whiteboard: nsbeta3-
Putting brackets in - from nsbeta3- to [nsbeta3-] to get jud's query to work
Whiteboard: nsbeta3- → [nsbeta3-]
Target Milestone: M20 → Future
Fuck it.  I'm going to whack extensions/ so hard it'll spin.

(Though I won't go out of my way to break them, I don't especially care if
wallet or cookie manager build and work when I'm done: people who care about
making them interface with Mozilla have had ample time to clean this mess up.  I
just care that we don't need the extensions/ stuff to build Mozilla, because
that's the contract of the directory, and lots of people want to build without
this code for a variety of reasons.  I guess someone who really cares about the
current wallet and cookie management code can fix that stuff in the MFuture.)
Assignee: dp → shaver
Severity: trivial → major
Keywords: arch
OS: Linux → All
Target Milestone: Future → M18
When I say ``whack extensions/'', I really mean ``whack all the things with
bullshit dependencies on extensions/, and which are not themselves extensions''.

Sorry for the confusion.
As one step towards this, I have some changes to abstract out our
security/crypto interface.  What I'm trying to do is provide interfaces for ssl
and crypto functions in a way not specific to PSM.  We can then export these
headers from some location other than extensions/psm-glue (I'm using
security/base), thus not making any part of the tree explicitly dependent on

Since patch seems to have problems patching's in multiple
directories, I've split this up into:

mozilla.patch   (which has the rest of the changes)
security-base.tar.gz  (contents of security/base)

I'm rolling all of these into a tarball which I'll attach next.
(note: bugzilla doesn't indicate it very well, but the above attachment should
be saved as a .tar.gz file)
Bob, could someone on your team comment (soon!) on bryner's approach?  I like
it, a lot, and I'd like to get it in soon so that we can remove dependencies on

Right now, I'm working on refactoring the wallet interfaces into ``form
management'' and ``password management''.  The former will live in
layout/public, and the latter in netwerk/public.  That'll let us disentangle the
build-time wallet dependency from the layout and networking code, for starters.

M18 is going to be a close thing, but it's not like anyone actually cares about
this. =/
See bug

Everything else should have been abstracted enough to replace cartman.
What do you mean by ``replace cartman''?  There are references to
nsIPSMSocketInfo in the core networking code, which comes from
extensions/psm-glue.  I'm actually not as worried about being able to replace
cartman as I am about being able to build the system without anything that
purports to be an ``extension''.

(Though bryner's patch makes the replacing of cartman pretty clean, too, for
those who would be motivated to do so.)

bryner: very slick clean-up, r,

Module owners should give a timely r=, or this patch will go in without their 
explicit review.  It fixes an ownership lapse, namely: making non-extension code 
depend on extensions/psm-glue.  It's not the end of the world, so owners should 
not get all defensive; but it is a mistake, and bryner's patch rectifies it as 
far as I can tell.

Same spiel for wallet and cookie dependencies from core/embeddable/non-extension 
parts of the codebase.

Unless there are objections from module owners, I'm going to check this in
tomorrow evening (Saturday).
Adding a couple random psm people to CC--

Do you guys have any objection to glue code going in under
shaver's patch looks good to me. r=valeski
The above patch adds nsIPasswordManager and such to netwerk/base/public, because
we use passwords for accessing network content, and I believe that netwerk
should define and publish the interface through which it is
controllable/extendable/pluggable, rather than having a compile-time dependency
on wallet/.

I have not yet adapted wallet to the new interface; I'll do that, if at all,
when all the other, similar changes are in place and I can do one big update.
I've got a bunch of new patches coming (mailnews, xpfe, imglib, etc.), and some
questions to go along with the phase after that:

 * OJI guys: does the OJI plugin directly call the cookie service, or does it
   use the PluginHost's cookie interface?  If you tell me it's the latter, I'll
   be very happy

 * Ben and the appcore police: the only remaining dependency on wallet that's in
   the core code is in the nsBrowserInstance appcore.  Can I get a hand from
   someone to whack that thing?  Should I just tear out the wallet stuff, and
   have the wallet UI retrofitted to use overlays for menu item addition, and
   modern XPConnect techniques to talk to the service itself?  The latter is
   very tempting indeed.

 * Brendan: are you still interested in adapting the cookie manager to use my
   politically-correct nsIContentPolicy hooks?  My patches trim out libimg's
   cookie manager calls (though somewhat superficially), so they'll be needed
   if we want image-filtering to survive.
We'll want image filtering to survive, hooked up via a proper XPCOM interface 
and not a #include.  Let me know when I should apply the latest consolidated 
patch and get busy.

At the moment, OJI has no dependencies on cookie.
edburns: Thank you for the great news.

Brendan: you can start making the cookie manager (sigh) do its image-filtering
through the nsIContentPolicy at your leisure; the hooks needed for that have
been in the tree for a while, awaiting your tender loving care.

Bryner/Pavlov: I need your security stuff in the tree to make sure that this is
all kosher.  Can I get an ETA?
My first patch above (for netwerk) is in the tree.

More patches coming ASAP.
The parade continues.

mailnews is more of the same as netwerk, really.

xpfe is removal of some wallet/cookie stuff, though the Big Appcore Mess lives.

netwerk is an updated interface, to pull in the prompting stuff.

alecf, jud, ben: I await your r.  brendan, I'd like some more a.

(The wallet and cookie functionality will be somewhat-to-completely broken until
this surgery -- and the work to make the wallet code use new interfaces -- is
complete.  Stay tuned!)
pnunn, alecf, tor: welcome to the party! BYOr=.
The plan as far as nsIBrowserInstance sounds fine to me. Several of us have been 
wanting to kill that for some time now and the wallet methods are among some of 
those in the way of that effort. Ditto the overlaying mechanism. If possible, 
the methods should be shifted into some wallet interface, and they can be called 
from overlayed UI. 
Patch coming up to stub out those methods, for starters.  I think those methods
should probably migrate over to the ever-shrinking nsIWalletService, but I'm not
yet planning how to put the wallet pieces back together.
Hey brendan: can I get a #46423-style blanket approval for this work.  (Not that
I see any mention of approval at all in that bug, but I'll be charitable and
presume it's been granted -- by Netscape marketing, natch! -- as per tinderbox.)
No blanket approvals yet, especially if there is a conflict between finishing
this overdue cleanup, and a contributor making a branch for a commercial product
that wants to include the "feature", implemented however.  Either we restore the
broken wallet functionality fast, or someone (shaver should help, at least) must
back out the partial cleanup from the branch.

What branch?  I'm up-to-date with my .seamonkey, and see no mention
of any such thing, though I've heard bits of back-room plans about it.

If wants my help backing out these changes on their branch, I'll
happily drive bonsai for them.  I have to admit that I'm surprised to have heard
only now, from, about's wallet concerns, when
there are a dozen people on this bug, several of whom have
expressed their support for this plan, and none of whom have objected.

I know that the embedding crowd tried to nominate this for nsbeta3, and some of
them have (privately) expressed their support for this, so I'm having a hard
time getting an understanding of what wants.  Does someone want to comment here?
we need to permit building w/ out the extensions dir (as the bug summary
indicates). This should be done w/ out breaking existing functionality IMO.
``Without breaking'' is a pretty tall order, but ``with repair afterwards''
isn't too bad at all.  Are people willing to help with that?  I know brendan has
signed up to fix the cookie manager/image-blocking stuff.

The only other big pieces of the repair are:
 - change wallet UI to call nsIWalletService directly, rather
   than using the (soon-to-be-elided) appcore hooks
 - teach wallet about the new interfaces (2 hours work, tops)
 - move wallet into overlays, like other optional components

I think that's it.  I'll even do it all, if people insist, as long as people can
put up with my travel schedule's imposition of delays.
No, dammit, we're trying to ship something here. "With repair afterwards" would 
be perfectly fine months ago or in a week or two after Netscape branches. Why 
right now? This was not so broken that we needed this kind of destabalization 
this week.

So now what--is it easier to go forward or go back? Your comments about the 
remaining tasks and your travel schedule were not reassuring compared to the 
ease of getting bonsai to spit out the cvs commands to back this stuff out.
If I could go back in time to several months ago, I would certainly do so, but I

Why not after you branch, you ask?  Are you here to tell us when that will be,
and under what terms?  I know nothing concrete about's branch plans
-- only rumour and speculation -- but I _do_ know that many people within the
Mozilla community, including some of your coworkers, want this bug
fixed to serve their needs.  When will Netscape branch?  For how long?

I want to write something here about ``we're trying to ship'' vs. what I see
every day being piled into the tree by folk with weak review and
weaker (IMO) approval, but I couldn't find anything that communicated how
infuriated, frustrated and saddened I am, and yet didn't contain too much
profanity.  So you'll just have to use your imagination.
Mike: You're well aware of bug 46859 which removes dependencies on 
nsIWalletService, right? The diffs are in there -- just waiting for someone to 
verify it.
I'm reading the diffs for that now, Warren, thanks for the heads-up.

I still see lots of 
    PRUnichar * prompt_string = Wallet_Localize("PromptForPassword");
in the psm-glue code -- does that not have a wallet dependency?

More comments in that bug, about that diff, coming.
Ok, I didn't remove all wallet dependencies, just the ones in necko 
(ForgetPassword). The idea here was to make it transparent whether wallet was in 
the chain or not between you and the dialog code.
shaver: we are completely with you on what the right fix is here. The issue was 
just about the timing that didn't turn out to be so good since we are really 
pressed for time (and stability) on the PR3 deadline. While we all from the 
bottom our heart appreciate the work you and several mozilla contributors are 
doing, we have to still "do the right thing" for weighing fixes against the 
risk. And as it turns out in this case, we clearly had regression. Based on that 
alone I have to request you to back this out. Thanks for understanding and for 
contributing a much needed cleanup of the codebase.
Trying to build moz, I keep on hitting this bug:

../../../../../mozilla/extensions/psm-glue/public/nsIPSMComponent.idl:27: can't
open included file nsISecurityManagerComponent.idl for reading

input callback returned failure
make[3]: *** [_xpidlgen/nsIPSMComponent.h] Error 2
make[3]: Leaving directory
make[2]: *** [export] Error 2
make[2]: Leaving directory
make[1]: *** [export] Error 2
make[1]: Leaving directory
make: *** [export] Error 2

I think the most recent update
for nsIPSMComponent.idl is fishy. There is no nsISecurityManagerComponent.idl to
be found anywhere in CVS...

The bug occurs when building with default extensions as well as when including
all extensions.
Are you using to check out?
No, I do a full SeaMonkeyAll checkout, and an update in the created mozilla
tree. I build in an objdir (../BUILD/mozilla). This used to work flawlessly up
until a couple of days ago...
If it did work, it was pure luck.  See
for the proper check out rules that have been in effect for over a year(?) now.

Shaver: I think I picked a nit about copy/paste codebloat in the paragraph or so 
needed to get from nsIURIs to the strings needed for shouldLoad or shouldProcess 
already.  Now over in bug 28327, mstoltz points out that he's adding yet more 
nsIScriptSecurityManager::CheckLoadURI calls, and it looks like nsIContentPolicy 
eats that interface method's dust.

Can we not consolidate code now that we have a few concrete cases to abstract?  
Should we combine services, or make a super-service that handles security and 
privacy decisions?

I agree; it would make sense to combine the security and privacy aspects of
content loading policy, so we aren't making two calls before every load. I'm
working on a rewrite of CheckLoadURI right now, so it's a good time to do this.
Can you suggest ways to improve the performance of this function?

Something Vidur mentioned about these security checks: as it stands, we have to
find everywhere in the code where a network load is initiated by web content
(basically, everything but the initial page load). The safer way to do this
would be to put the check as low as possible, say in the AsyncRead calls which
always occur as part of a load, and require every call site to indicate what its
security policy should be. This may be infeasible now (it'd be a huge change)
but it would basically force code writers to consider this issue. We also talked
about the very thing Brendan mentioned in 28327 as part of Shaver's content
policy hooks: a parameter which gives the type of URI load. I'm probably going
to have to add this to CheckLoadURI to do some of the things i want to do with it.

It definitely makes sense to combine our security and privacy efforts, at least
at the implementation level.
Blocks: 58372
Depends on: 61219
Milestone 0.8 has been released. We should either resolve this bug or update its
Target Milestone: M18 → mozilla0.9
It breaks my heart (especially because we're doing work in other bugs that
undoes some of the steps here), but...0.9.1.
Target Milestone: mozilla0.9 → mozilla0.9.1
(Someone less lame than me should feel free to take this bug from me.)
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Asa just told me that we shipped 0.9.2!  Who knew?
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Depends on: 92377
Target Milestone: mozilla0.9.3 → mozilla0.9.4
->0.9.5 since we have run out of time
Target Milestone: mozilla0.9.4 → mozilla0.9.5
I don't really know when I will get this done, so I'm removing the lying
milestone marker.
Keywords: helpwanted
Target Milestone: mozilla0.9.5 → ---
Depends on: 113515
Depends on: 113519
Depends on: 113538
Depends on: 113540
Making this a meta bug for extensions/ build time dependencies.
Assignee: shaver → seawood
Priority: P3 → P1
Hardware: PC → All
Summary: Cannot build browser if extensions dirs are missing → [META] application should not require extensions/
Whiteboard: [nsbeta3-]
Summary: [META] application should not require extensions/ → [META] build should not require extensions/
Target Milestone: --- → mozilla0.9.9
Keywords: mozilla1.0+
Keywords: mozilla1.0
Now that peterv has checked in the fix to bug 92377, this bug seems to be fixed.
 myotonic is green, and my own build with the following options:

ac_add_options --disable-crypto
ac_add_options --disable-mathml
ac_add_options --disable-svg
ac_add_options --disable-extensions
ac_add_options --disable-jsd
ac_add_options --disable-ldap
ac_add_options --disable-mailnews
ac_add_options --disable-xprint
ac_add_options --disable-accessibility
ac_add_options --disable-bidi

works.  These are the same options as myotonic uses.

However, this should also be tested using --disable-extensions while enabling
everything else (to ensure that mathml, crypto, mailnews, bidi, accessibility,
etc., don't have incorrect dependencies on extensions).
Are we done here?  14889 seems like an independent perf bug.  True?

cls, can we escalate tinderbox no-extensions build failures (shrike and
myotonic, IIRC) to now?

Yes, it looks like we're done here & 14889 is unrelated to the
--disable-extensions issue.   Shrike is building with all extensions (as are
most ports) & myotonic is building without any extensions.  Yes, sheriffs should
know that bustage on myotonic is tree-stopping.  Should we just move it to the
main page?
Closed: 20 years ago18 years ago
No longer depends on: 14889
Resolution: --- → FIXED
We could move myotonic to the main page, but let's propose that to

As dbaron pointed out, we might want some machine to reconfigure itself to test
build-from-scratch cycling through all permutations of extensions -- that would
be fun!  This JS or equivalent gmake fu might help:

function permuter(a) {
  if (!a || a.length == 0)
    throw "can't permute empty sequence";
  if (a.length == 1)
    return a;
  var b = []
  for (var i = 0; i < a.length; i++) {
    var elem = a.slice(i, i+1);
    var rest = permuter(a.slice(0, i).concat(a.slice(i+1)));
    for (var j = 0; j < rest.length; j++)
      b[b.length] = elem.concat(rest[j]);
  return b;

print(uneval(permuter([1, 2])));
print(uneval(permuter([1, 2, 3])));
print(uneval(permuter([1, 2, 3, 4])));

Substitute extension names for the numbers 1-4.

that would be so cool, but should probably be a separate bug.  cc'ing mcafee
since he's been hacking the unix tinderbox scripts the most lately and might
find this interesting.
there's now a bugzilla component for tinderbox changes, perhaps you should use 

personally i'd prefer to build all extensions and report all failures instead 
of permuting.

I think mcafee or I could modify the build/reporting scripts to send back build 
failure notifications asap and then continue building and reporting them.  
assuming bustage breaks multiple extensions at once and that the build cycles 
aren't short or free, it's imo cheaper to report all the extension failures so 
that people can work on fixing all of them at once instead of waiting (consider 
the number of cycles it took to get the mac trees green [ignore the fact that 
the affected code wasn't extensions])
Reopening, we should not have this discussion in a fixed bug log.

be: I don't like the permute thing, it's too confusing, people
need something easy & brain-dead stupid.  If there's a bunch of
combinations we want to cover, then ok.  I'd rather cover the
cases that matter in a static way, this is much more maintainable
for developers and sheriffs.

If we want to keep the --disable-extensions option building,
let's turn it on for one of the main tinderboxes that are
already on the main page.
Resolution: FIXED → ---
Open a separate bug for the tinderbox discussion or move it to the newsgroups.

FWIW, I don't think the permutation based tinderbox would by us much.  I tried
something similar a couple of years ago for random build options combinations. 
I didn't seem to have much effect on tracking bustage & getting it fixed but
that could have been because it was on the Ports page as well.

I did test to make sure that we could still build on win32 & linux with
everything enabled but extensions.
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
mcafee: yeah, the compleatist in me was out for a jog.

timeless: we already test all extensions on one tinderbox, IIRC.  We are now
talking about (someone mailing to propose that!) a main page
tinderbox test the no-extensions case.

I'm ok with catching bogo-dependencies among extensions by hand, or by luck, for

Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.