Closed
Bug 165626
Opened 22 years ago
Closed 22 years ago
Package type ahead find in default install
Categories
(SeaMonkey :: Find In Page, defect, P2)
SeaMonkey
Find In Page
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.2alpha
People
(Reporter: aaronlev, Assigned: aaronlev)
References
Details
Attachments
(1 file)
9.53 KB,
patch
|
curt
:
review+
aaronlev
:
superreview+
aaronlev
:
approval+
|
Details | Diff | Splinter Review |
Type ahead find is becoming mature enough to get packaged with the default installer. This is the bug for that.
Assignee | ||
Updated•22 years ago
|
Blocks: isearch
Status: NEW → ASSIGNED
OS: Windows 2000 → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla1.2alpha
Assignee | ||
Comment 1•22 years ago
|
||
Yeek, for some reason, typeaheadfind won't work in resulting installed Mozilla. What's wrong with this thing?
Assignee | ||
Comment 2•22 years ago
|
||
It works after I manually run regxpcom in the installation directory. I don't know why that's not happening automatically.
Assignee | ||
Comment 3•22 years ago
|
||
This does work after all, yay! I was running into bug 162593. Seeking r=
Assignee | ||
Comment 4•22 years ago
|
||
Asa really wants this for 1.1a, so hopefully we can get this r=/sr=/a='d in the next day or two. Curt?
Component: Keyboard Navigation → XSLT
Comment 5•22 years ago
|
||
Comment on attachment 97278 [details] [diff] [review] This should work, shouldn't it! ? r=curt
Attachment #97278 -
Flags: review+
Comment 6•22 years ago
|
||
Comment on attachment 97278 [details] [diff] [review] This should work, shouldn't it! ? As mentioned on IM I think it's overkill to make this a separate component, but if you want to go that way it looks pretty good. This will not get installed on a commercial build, btw, you'd have to duplicate nearly everything but the package list changes in the ns tree. Once moved into browser.xpi (just move lines around in the package lists and add the chrome registration call to the browser.jst files) you should remove the typeaheadfind.js default prefs and merge into all.js to avoid slowing down startup. You've said you'll file a bug on those improvements. >Index: packager/mac/typeaheadfind.jst >=================================================================== >+var gVersion = "0.1"; Since this is a binary component it's probably mozilla-version-specific. Probably better to simply use the overall browser version. But that'll happen automatically when this gets moved into browser.xpi so you can leave this for now. >+err = addDirectory("", gVersion, "bin", fProgram, "", true); On the Mac this has to be "viewer" instead of 'bin' to match the build directory (as seen in the package lists). >+registerChrome(LOCALE | DELAYED_CHROME, getFolder("Chrome","typeaheadfind.jar"), "locale/en-US/typeaheadfind/"); In the region-pack scripts Tao registers a placeholder CONTENT contents.rdf, is this required here to make this work? You said you had one in your build, too, but it's not registering one here. On the other hand you said you tested without it and it worked, so I'm good with that. >+err = addDirectory("", gVersion, "bin", fProgram, "", true); You could simplify this to err = addDirectory("", "bin", fProgram, ""); if you wanted. Doesn't matter to me though >+if (getLastError() == SUCCESS) { >+ err = performInstall(); >+ logComment("performInstall: " + err); >+} else { >+ cancelInstall(err); >+} If you get an error from the registerChrome() call you will return the addDirectory error, which very well might be success. But if addDirectory chrome registration will *definitely* fail with a file-not-found error, which is probably not as useful as knowing why we couldn't write the file out (permissions? out of space?) at the very least do err = getLastError(); if (err == SUCCESS) ... etc but consider checking err before bothering with the registerChrome() call
um, given the various regressions and bugs that typeahead has caused recently, i don't think it's ready :-) as for getting this into 1.1a, you're a few months late :-). if you want lots of feedback from 1.2a that's a different story, but let's be sure we don't know of any major bugs in it before we decide to flip the switch.
Component: XSLT → Installer: XPI Packages
Assignee | ||
Comment 8•22 years ago
|
||
Sorry, I mean 1.2a. Dan, I have these changes - do you want to see a new patch?
Comment 9•22 years ago
|
||
sr=dveditz given to modified patch reviewed on Aaron's machine. This is a short-term solution though, this should get rolled into the main install (bug 166712).
Comment 10•22 years ago
|
||
Comment on attachment 97278 [details] [diff] [review] This should work, shouldn't it! ? doesn't make a lot of sense to have it in the zips and not in the installers. a=asa (on behalf of drivers) for checkin to 1.2a
Assignee | ||
Comment 11•22 years ago
|
||
Comment on attachment 97278 [details] [diff] [review] This should work, shouldn't it! ? Has sr=dveditz, a=asa
Attachment #97278 -
Flags: superreview+
Attachment #97278 -
Flags: approval+
Assignee | ||
Comment 12•22 years ago
|
||
Should show up in tomorrows Linux and Windows installers.
Assignee | ||
Comment 13•22 years ago
|
||
Marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 14•22 years ago
|
||
The .xpi is part of the linux sea installer on linux, but it doesn't actually install components/libtypeaheadfind.so. Even after I point it at the xpi and it reports no errors, the library isn't there. I do have defaults/pref/typeaheadfind.js. Should I reopen this bug, or is there another bug tracking the issue? If I unzip the xpi, I see: creating: bin/ creating: bin/components/ inflating: bin/components/libtypeaheadfind.so inflating: bin/components/typeaheadfind.xpt creating: bin/chrome/ inflating: bin/chrome/typeaheadfind.jar creating: bin/defaults/ creating: bin/defaults/pref/ inflating: bin/defaults/pref/typeaheadfind.js inflating: install.js
Assignee | ||
Comment 15•22 years ago
|
||
Aha, it's getting typeaheadfind.js from browser.xpi, somehow it got in there too. For some reason, when setup.exe runs this line in xpi.c: hrResult = pfnXpiInstall(szArchive, "", 0xFFFF); for typeaheadfind.xpi, it comes back with a successful result but nothing was unzipped. How do the bin\ directories get changed to the right name as things get unzipped?
Assignee | ||
Comment 16•22 years ago
|
||
Awww... man .... I managed to screw up and not checkin the new typeaheadfind.jst files. It was substituting some other install.js for mine, but there was no error.
Assignee | ||
Comment 17•22 years ago
|
||
Ah, I noticed a bug with the installer, we package all the .js prefs files twice, once in browser.xpi and once in the specific package xpi. Doesn't seem to hurt anything.
Comment 18•22 years ago
|
||
It hurts performance, because a browser-only install then has to parse extra pref files at startup. But then supposedly profile migration doesn't work right if the mailnews pref defaults aren't present, and then if the user later wants to add mail there is no way to migrate mail. Sounds weak to me, but that was the excuse at the time.
Comment 19•22 years ago
|
||
> It hurts performance, because a browser-only install then has to parse extra > pref files at startup. by how much? > But then supposedly profile migration doesn't work right > if the mailnews pref defaults aren't present, and then if the user later wants > to add mail there is no way to migrate mail. Sounds weak to me, but that was > the excuse at the time. if it is really a perf hit, here's an idea: at installer time, if I install browser and mail, cat all.js mailnews.js editor.js > foo.js and now you've only got one file to parse at startup. but we need the mailnews defaults. if you migrate your 4.x profile, we need to have the defaults in mailnews.js or as dan says, running mail later will fail. common scenario: 1) install mozilla 1.0 browser only, migrate 4.x profile 2) decide you like it, so when 1.1 is out, you get browser and mail. another alternative is to fix when / how we do migration. but our resources are better spent elsewhere.
Comment 20•22 years ago
|
||
sorry, I mean: "if it is really a perf hit, here's an idea: at installer time, if I install browser and mail, cat all.js mailnews.js editor.js > foo.js" in the tree have seperate js files. but in the installed default/prefs dir, have one. so we pay for the hit at install time, not at startup each time.
Comment 21•22 years ago
|
||
I've taken the perf issue to bug 168515, and assigned to dveditz.
Comment 22•21 years ago
|
||
lob- Sarah, are you familiar with this- enough to verify?
QA Contact: ktrina → sairuh
Comment 23•21 years ago
|
||
find as you type (aka, type ahead find) is now part of mozilla builds. tested with 2003.01.31.08 mozilla bits on linux rh8.0.
Status: RESOLVED → VERIFIED
Updated•16 years ago
|
Product: Core → SeaMonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•