Closed Bug 102574 Opened 23 years ago Closed 23 years ago

enable document inspector in the default builds

Categories

(SeaMonkey :: Build Config, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla0.9.7

People

(Reporter: asa, Assigned: hewitt)

References

Details

Attachments

(1 file, 1 obsolete file)

I would like to see the document inspector enabled in the default nightly and Milestone builds.
Blocks: 101793
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.5
Attached patch patch to build inspector (obsolete) (deleted) — Splinter Review
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).
cc'ing Seawood since he's probably a good person to review this.
What's the point of building it if we don't distribute it via the installer?
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.
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. :)
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.
Hopefully more people will be able to help review this during the 0.9.6 cycle...
Target Milestone: mozilla0.9.5 → mozilla0.9.6
No longer blocks: 101793
Depends on: 105277
So, jag and hyatt have reviewed the code for me and given the thumbs up. Shall we flip the switch?
Joe, get jag to put his r= and hyatt to put his sr= in bug 105277 and we're ready to go.
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.
Target Milestone: mozilla0.9.6 → mozilla0.9.7
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.
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.
Attachment #51597 - Attachment is obsolete: true
Attached patch new patch (deleted) — Splinter Review
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 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=.
Attachment #58662 - Flags: superreview+
Comment on attachment 58662 [details] [diff] [review] new patch wow! installer patches, too. nice job, joe.
Attachment #58662 - Flags: review+
fixed. woohoo!
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
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.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: