Closed
Bug 190560
Opened 22 years ago
Closed 22 years ago
xpcom is unable to always load .js component file
Categories
(Core :: XPCOM, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: ssu0262, Assigned: dougt)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
ssu0262
:
review+
alecf
:
superreview+
jesup
:
approval1.3b+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•22 years ago
|
||
nor am I.
how are you registering this .js file?
Assignee | ||
Comment 3•22 years ago
|
||
note that dan's comment was about the "component" attribute of this bug.
Assignee | ||
Comment 4•22 years ago
|
||
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.
Flags: blocking1.3b?
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.
Assignee | ||
Comment 6•22 years ago
|
||
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.
Assignee | ||
Comment 7•22 years ago
|
||
ignore that last comment.
Assignee | ||
Comment 8•22 years ago
|
||
duh! found the problem.
Assignee | ||
Comment 9•22 years ago
|
||
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?
Assignee | ||
Updated•22 years ago
|
Attachment #112836 -
Flags: superreview?(alecf)
Attachment #112836 -
Flags: review?(ssu)
Comment 10•22 years ago
|
||
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+
Assignee | ||
Comment 11•22 years ago
|
||
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.
Updated•22 years ago
|
Flags: blocking1.3b? → blocking1.3b+
Reporter | ||
Comment 12•22 years ago
|
||
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-
Assignee | ||
Comment 13•22 years ago
|
||
Comment on attachment 112836 [details] [diff] [review]
patch v.1
grumble.... wrong patch.
Attachment #112836 -
Attachment is obsolete: true
Assignee | ||
Comment 14•22 years ago
|
||
this is the real deal.
Assignee | ||
Comment 15•22 years ago
|
||
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.
Attachment #112864 -
Flags: superreview?(alecf)
Attachment #112864 -
Flags: review?(ssu)
Comment 16•22 years ago
|
||
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 17•22 years ago
|
||
Comment on attachment 112864 [details] [diff] [review]
patch v.2
sr=alecf
Attachment #112864 -
Flags: superreview?(alecf) → superreview+
Comment 18•22 years ago
|
||
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+
Reporter | ||
Comment 19•22 years ago
|
||
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-
Assignee | ||
Comment 20•22 years ago
|
||
ssu - why are you missing this patch on some error that you are seeing in a
totally different area?
Reporter | ||
Comment 21•22 years ago
|
||
because xpinstall requires xpcom, and xpinstall is used by the native installer.
Assignee | ||
Comment 22•22 years ago
|
||
never mind... ssu found the problem.... i shouldn't have default the return
result to NS_ERROR_UNEXCEPTED. ssu say r=
Reporter | ||
Comment 23•22 years ago
|
||
r=ssu for changing the default return value.
Reporter | ||
Comment 24•22 years ago
|
||
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+
Assignee | ||
Comment 25•22 years ago
|
||
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
Closed: 22 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 26•22 years ago
|
||
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.
Description
•