Seth and I found a problem with xpcom trying to load a .js component from the mozilla's component dir (as opposed to the GRE's component dir). We were trying to fix bug 190465, and noticed that nsDownloadProgressListener.js would only load consistently when it was located in the GRE's component dir. If it was in the mozilla's component dir. It would only load once, then the feature would no longer work if you quit and restarted the browser. Even this one time was not always reproduceable. Both seth and I were experiencing this, so I doubt it's something with my system in particular. Seth tested out other features that had .js components in the mozilla's component dir. They seem to have loaded fine. So I'm not sure exactly what's going on here.
nor am I. how are you registering this .js file?
Not the registry, just XPCOM
Component: XPCOM Registry → XPCOM
note that dan's comment was about the "component" attribute of this bug.
this works for me. ssu/seth, can you reproduce this problem for me on monday? If this is a problem, I wanna know right away.
here are the steps with a new build that *contains* the fix to bug 190465: 1) make sure mozilla and GRE are not installed 2) install mozilla (default settings is what I used) 3) notice that doing a "File|Save Page As" (and actually saving the file) works. 4) quit mozilla when it runs at the end of the install 5) delete [mozilla]\components\*.dat 6) *move* the following files from [gre 1.3b]\components to [mozilla]\components: nsProgressDialog.js nsHelperAppDlg.js nsDownloadProgressListener.js 7) start mozilla 8) notice that doing a "File|Save Page As" (and actually saving the file) does *not* work. Dougt, if the steps above still do not reproduce this bug, I can swing by your cube and help you reproduce this.
where is xpc3250 in the install? I do not see this file in any of the compreg.dat files ssu has sent me via email.
ignore that last comment.
duh! found the problem.
Created attachment 112836 [details] [diff] [review] patch v.1 Keep track of the component loaders. If there is an additional component loader added during the registration of gre components, we autoreg the application components directory again (for the new non-native loaders only). I am pretty sure that this works. ssu, can you give it a swing?
Comment on attachment 112836 [details] [diff] [review] patch v.1 generally this looks fine, but can we keep the DEBUG_dougt stuff? I think we have enough registration spew already. Also, I see lots of NS_COM/NS_EXPORT stuff - is that intentional? necessary? sr=alecf with the DEBUG_dougt left, and an explanation for the NS_EXPORT stuff
Attachment #112836 - Flags: superreview?(alecf) → superreview+
Comment on attachment 112836 [details] [diff] [review] patch v.1 okay to the printf's. Regarding the NS_EXPORT stuff, the function that I am removing this from isn't used outside of XPCOM. There is no reason to export it, nor (i guess) no real reason include it with this bug. It was just in my tree at the time I took the diff. If it is all the same, I would like to check this part in too.
Comment on attachment 112836 [details] [diff] [review] patch v.1 xpinstall now errors out from AutoRegisterComponentByLoaderIndex(). It was not going into the for() loop. fromLoaderType is 0. mNLoaderData is 1.
Attachment #112836 - Flags: review?(ssu) → review-
Comment on attachment 112836 [details] [diff] [review] patch v.1 grumble.... wrong patch.
Attachment #112836 - Attachment is obsolete: true
Comment on attachment 112864 [details] [diff] [review] patch v.2 sorry about that, the only different between the two patchs is alecf's suggestion regarding the printf, and i made that AutoRegisterByLoaderIndex more encapsulated by not exposing the loader id in the method and called this new method AutoRegisterNonNativeComponents. This makes more sense.
Comment on attachment 112864 [details] [diff] [review] patch v.2 >+ // registers only the files in spec's location by loaders other than the >+ // native loader. This is an optimization method only. >+ nsresult AutoRegisterNonNativeComponents(nsIFile* spec); the comment is wrong: if spec is null you intentionally check some other directory :)
Comment on attachment 112864 [details] [diff] [review] patch v.2 sr=alecf
Attachment #112864 - Flags: superreview?(alecf) → superreview+
Comment on attachment 112864 [details] [diff] [review] patch v.2 Approval granted with the proviso that you need to get r= from ssu (or someone else up on what's going on here with GRE/etc).
Attachment #112864 - Flags: approval1.3b+
Comment on attachment 112864 [details] [diff] [review] patch v.2 still not working for xpinstall being triggered via xpistub.dll. I'll talk with you off line to help you reproduce this.
Attachment #112864 - Flags: review?(ssu) → review-
ssu - why are you missing this patch on some error that you are seeing in a totally different area?
because xpinstall requires xpcom, and xpinstall is used by the native installer.
never mind... ssu found the problem.... i shouldn't have default the return result to NS_ERROR_UNEXCEPTED. ssu say r=
r=ssu for changing the default return value.
Comment on attachment 112864 [details] [diff] [review] patch v.2 contingent on changing the default return value to NS_OK per our talk.
Attachment #112864 - Flags: review- → review+
Thanks everyone. Checking in components/nsCategoryManager.cpp; /cvsroot/mozilla/xpcom/components/nsCategoryManager.cpp,v <-- nsCategoryManager.cpp new revision: 1.36; previous revision: 1.35 done Checking in components/nsComponentManager.cpp; /cvsroot/mozilla/xpcom/components/nsComponentManager.cpp,v <-- nsComponentManager.cpp new revision: 1.219; previous revision: 1.218 done Checking in components/nsComponentManager.h; /cvsroot/mozilla/xpcom/components/nsComponentManager.h,v <-- nsComponentManager.h new revision: 1.78; previous revision: 1.77 done Checking in components/nsRegistry.cpp; /cvsroot/mozilla/xpcom/components/nsRegistry.cpp,v <-- nsRegistry.cpp new revision: 1.72; previous revision: 1.71 done
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
this fix seems to be working. I verified by doing a manual fix of bug 191048. the download manager dialog shows up and works after I move nsDownloadProgressListener.js from GRE to Mozilla.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.