102574 - enable document inspector in the default builds

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect, P2)

Description

[Asa Dotzler [:asa]]

I would like to see the document inspector enabled in the default nightly and
Milestone builds.

Comment 1

[Joe Hewitt (gone)]

Attachment: patch to build inspector

Comment 2

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Um... do we really want the Inspector to be included in the installers?

Also I was wondering, shouldn't we make it possible to NOT build Inspector? I
can see how to do that on the Mac, not sure about Unix, but there definitely is
no way to do that on Windows with the patch as is... How about playing with
environment variables. Set MOZ_ENABLE_DOM_INSPECTOR in win32.inc and in
extensions/makefile.win see if that is set before building... (or something like
this, please check with Windows build gurus). 

Comment 3

[Dawn Endico]

cc'ing Seawood since he's probably a good person to review this.

Comment 4

[Joe Hewitt (gone)]

What's the point of building it if we don't distribute it via the installer?  

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Useful for debugging, expose to more testers before enabling for all users.

I have some questions: has the DOM Inspector undergone a formal review and
super-reviews process? What about security review? Unless it satisfies all of
these, I would not want to see it in the installers. In fact, I am a bit
uncertain about turning it even on before these...

Stability & correctness are also issues I would like to get some attention
before throwing it out for everyone. Have someone run it with Purify, and fix
any problems found before putting it in the installers. Also fix any outrageous
leaks before putting it in the installer.

I would be thrilled to have it BUILD by default, though, because I use it for
debugging myself. Also, if it was built by default, it would help with
bitrotting at least a little bit.

Comment 6

[hacker formerly known as [e-mail]]

Wrt the bitrotting, nearly all of the unix tinderboxes build with all of the
extensions enabled, including inspector. 

On unix, you have to specify --with-extensions=<all options but inspector> to
turn this off.  Yes, it's lame.

This is another one of those things that needs collective agreement between
mozilla.org & the build team before turning on by default.  The superreviewers &
security reviews heikki mentioned would probably be good as well. :)

Comment 7

[Joe Hewitt (gone)]

Not every line of code in Inspector has been reviewed.  If there are people out
there who are interested in signing up to review the DOM Inspector code, perhaps
we could split up the files into small groups and have people review only those
parts.

I agree that we should do some QA on Inspector before sending it out to the
world, I am just wary about how much of this would actually get done if this
component was NOT in the installed builds, as very few people will even get the
chance to test it.

Anyway, for the moment ignore the installer parts of my patch, and let's just
review and land the parts to turn this on in the builds for now.

Comment 8

[Joe Hewitt (gone)]

Hopefully more people will be able to help review this during the 0.9.6 cycle...

Comment 9

[Joe Hewitt (gone)]

So, jag and hyatt have reviewed the code for me and given the thumbs up.  Shall
we flip the switch?

Comment 10

[Asa Dotzler [:asa]]

Joe, get jag to put his r= and hyatt to put his sr= in bug 105277 and we're
ready to go.

Comment 11

[Joe Hewitt (gone)]

I was hoping to have this in for 0.9.6, but I would feel more comfy turning this
on at the beginning of a milestone so that it has time to bake.  So, let's flip
the switch as soon as the tree opens for 0.9.7 and then I will have time to fix
bugs before it goes out in a milestone release.

Comment 12

[Joe Hewitt (gone)]

Tonight I landed a ton of fixes for DOM Inspector which will make it crash less,
run faster, and have less weird errors and assertions.  jag reviewed most of my
C++ code and says r=jag, and hyatt says sr=hyatt, so I think now would be a good
time to turn this on.

I don't know whose approval I need exactly, but if somebody from staff@mozilla
could give me a thumbs up, I will checkin the code for building and packaging
this baby.

Comment 13

[Jon Granrose]

whether it gets turned on or not by default to ship with optimized installer
builds is up to mozilla.org.  personally, I don't see why we'd want to ship it
but I don't do any DOM work, so I don't know how useful it may be.

ccing leaf since we'll need to know how to disable this in Netscape builds if
mozilla oks it for mozilla builds.

Comment 14

[Joe Hewitt (gone)]

Attachment: new patch

Comment 15

[Sean Su]

Comment on attachment 58662 [details] [diff] [review]
new patch

in the windows' config.it file, have Component DOM Inspector be C10= instead of
C11=.

Component QFA should be listed last.

Also I'm not familiar as to how this will affect the packages-static builds.  I
would defer that part of the review to Cathleen.

Are we even still doing static builds?	I noticed that venkman isn't in the
packages-static- files, bu t it is the config.it file.	This would make the
static build's installer to fail.

The other stuff looks good tho.

Comment 16

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 58662 [details] [diff] [review]
new patch

sr=shaver.  I'd like to see leaf or cls stamp in the bug with their r=.

Comment 17

[Daniel (Leaf) Nunes]

Comment on attachment 58662 [details] [diff] [review]
new patch

wow! installer patches, too. nice job, joe.

Comment 18

[Joe Hewitt (gone)]

fixed. woohoo!

Comment 19

[Asko Tontti]

inspector.xpi is missing for linux nightly builds (2001112906 and 2001112908).
This causes problems with the installer. For example complete install doesn't
work as expected.