Closed
Bug 258123
Opened 20 years ago
Closed 20 years ago
Cannot register own JavaScript Component with new interface
Categories
(Toolkit :: Add-ons Manager, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: patrick, Assigned: shaver)
References
Details
(Keywords: fixed-aviary1.0)
Attachments
(5 files, 3 obsolete files)
7.05 KB,
application/x-xpinstall
|
Details | |
7.20 KB,
application/x-gzip
|
Details | |
14.16 KB,
patch
|
Details | Diff | Splinter Review | |
16.11 KB,
patch
|
shaver
:
review+
benjamin
:
superreview+
|
Details | Diff | Splinter Review |
7.00 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3 With the latest Firefox and Thunderbird builds (Thunderbird >= 20040901), it is not possible anymore to correctly register a component written in JavaScript that provides a specific interface not previously known to FF / TB. Reproducible: Always Steps to Reproduce: 1. Install the attached XPI 2. Type chrome://enigmail/content/enigmailAbout.xul into the URL bar, or window.openDialog("chrome://enigmail/content/enigmailAbout.xul") in the JavaScript console Actual Results: On Mozilla 1.7 and Thunderbird 0.7, you'll see that the dialog prints success, i.e. the dummy component is correctly loaded. With the nightly FF and TB, you'll see an exception saying that Components.interfaces.nsIEnigmail does not exist, but that's what the XPI should provide.
Reporter | ||
Comment 1•20 years ago
|
||
Reporter | ||
Updated•20 years ago
|
Version: unspecified → 1.0 Branch
Reporter | ||
Comment 2•20 years ago
|
||
The attached XPI can be unpacked and built in mozilla/extensions
Comment 3•20 years ago
|
||
Maybe this is of some interest: If I start with a fresh profile and copy first the components/ files of the example xpi to the thunderbird components directory, then start thunderbird and install the xpi through the extension manager, the test case will succeed. If I just install the .xpi the test-case fails and it is not recoverable by copying the components/ files of the xpi archive manually to the thunderbird components directory anymore. For me this sounds as if components in the $HOME/.thunderbird/xxx/extensions/xxxx/components are not loaded, but registered only (since manually copying those files to the global components directory after that won't help and won't register any new components)
Comment 4•20 years ago
|
||
As pointed by alex@elbrus.com, this must be a duplicate of bug 257681. If you agree, please post the testcase# (157963) there and mark this bug as a dup of that?
Reporter | ||
Comment 5•20 years ago
|
||
I'm sorry, I searched for such a bug, but couldn't find one. *** This bug has been marked as a duplicate of 257681 ***
Status: UNCONFIRMED → RESOLVED
Closed: 20 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Comment 6•20 years ago
|
||
So I think this is correct (it works in my testing with the attached extension-test, which failed before my patch), because xptiInfoManager goes to some lengths to only reregister files that it doesn't already have data for, and additionally to make sure that duplicate interfaces are really identical. I've convinced myself that this specific reload case is safe. (It might well be safe in the general case, but I only did the mental gymnastics to track this part, so we should revisit it before, f.e., attempting this during no-restart extension install.)
Assignee | ||
Updated•20 years ago
|
Assignee: bugs → shaver
Status: RESOLVED → ASSIGNED
Resolution: DUPLICATE → ---
Assignee | ||
Comment 7•20 years ago
|
||
Comment on attachment 158260 [details] [diff] [review] expose interface for rebuilding search path I'd love to hear from the real xptinfo experts, though. I added this additional interface so that I wouldn't disturb the web services code, which implements nsIInterfaceInfoManager as well.
Attachment #158260 -
Flags: superreview?(jband_mozilla)
Attachment #158260 -
Flags: review?(dbradley)
Updated•20 years ago
|
Flags: blocking-aviary1.0PR+
Comment 8•20 years ago
|
||
*** Bug 257681 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 9•20 years ago
|
||
Comment on attachment 158260 [details] [diff] [review] expose interface for rebuilding search path This isn't sufficient; the extra dirs are being hidden from xptiinfomanager on non-registering starts of firefox (not the restart-to-install, but every one after that) because we don't go through nsXREDirProvider::RegisterExtraComponents, and mRegisterExtraComponents stays PR_FALSE. Now, I'm not sure why we're not just working from the same xpti.dat as before, but it's probably because the dir list is different. Alas... (Also, this patch is in the wrong bug, arguably.)
Attachment #158260 -
Attachment is obsolete: true
Comment 10•20 years ago
|
||
I needed shaver's patch after all for various reasons. But the end result is something that still doesn't work. Things are registered, but the NS_ERROR_XPC_CI_RETURNED_FAILURE error persists.
Comment 11•20 years ago
|
||
Background: When an extension is installed, the contents of its XPI file are inflated to: <dir>/extensions/{GUID}/ Shaver proposed the following: When an extension is installed or enabled, the contents of the extension's "components" directory are copied from: <dir>/extensions/{GUID}/ to <dir>/extensions/components/{GUID} This way, xptiInterfaceInfoManager would only have to know about one components dir, instead of several. When an extension is uninstalled or disabled, its {GUID} dir under <dir>/extensions/components would be removed. The reason shaver's patch is still required is that we can't tell the Component Registry or the xptiInterfaceInfoManager about the new <dir>/extensions/components dir until *after* the EM has been started and pending EM uninstall/install operations have been performed, otherwise CompReg etc may load components in that location and we will be unable to delete them (because they will be busy). So I needed a way to force the xptiInterfaceInfoManager to rebuild its search path (supplied by shaver's patch). The additional code in my patch is related to the fact that xptiInterfaceInfoManager, despite shaver's strenuous insistence (:p) does *not* seem to enjoy scanning recursive subdirs for components. I had to teach xptiInterfaceInfoManager how to look deeper into subdirs and to consider parent chains when testing for file containment. Finally, the remainder of my patch (in and around nsExtensionManager.js.in, nsAppRunner.cpp and nsXREDirProvider.cpp) is to do with getting rid of components.ini and simply using the "components set changed" flag as a trigger to call autoregister. So, a lot of code. But does it work? No. The Enigmail example throws the exception shaver was seeing earlier: JavaScript error: line 0: uncaught exception: [Exception... "Component returned failure code: 0x8 0570015 (NS_ERROR_XPC_CI_RETURNED_FAILURE) [nsIJSCID.createInstance]" nsresult: "0x80570015 (NS_ERROR_XPC_CI_RETURNED_FAILURE)" location: "JS frame :: chrome: //enigmail/content/enigmailAbout.xul :: enigAboutLoad :: line 29" data: no] possibly because without knowing it, my patch has recreated exactly the same scenario that shaver created earlier, just in a much more complicated way. I have to say, if it's not going to work, I prefer the simplicity of the original setup ;-) Now, I could be wrong and maybe there's something simple wrong with what I have that'd make it work. I now hand the batton to shaver to carry for the early morning, while I slumber.
Assignee | ||
Comment 12•20 years ago
|
||
I went deeper here, and tightened up the logic for search-path changes a bit, including support for skipping newly-missing dirs, which helps here. The dirs need to be missing at the end, because I'm not doing dir-index remapping work here, so I moved the plugins dir _before_ extensions. I don't think it's a problem, but you can try to convince me otherwise. One thing that made me a little nervous is appending the empty xptiFile, but I don't think we'll ever be asked for it, because the interface entries will be missing, so nothing will point to that index. This makes the test work for me, other than the fact that .value doesn't actually update. I can print and operate on the returned |a| fine, though, so I think there's something else at play here. (Also, moving the assignment to before the Components poking doesn't change things, so I call "SEP".)
Assignee | ||
Updated•20 years ago
|
Attachment #158296 -
Attachment is obsolete: true
Attachment #158260 -
Flags: superreview?(jband_mozilla)
Attachment #158260 -
Flags: review?(dbradley)
Assignee | ||
Comment 13•20 years ago
|
||
So it turns out that this bug was caused by: 1.27.2.1.4.36 <ben@bengoodger.com> 2004-08-31 22:01 Fix numerous Extension Manager/Update bugs, mostly 257304 but also: 247322 252543 256930 256960 256961 257045 257047 257179 reviews pending. Which moved the RegisterExtraComponents call down to _below_ the XPCOM autoregistration, which then meant that we violated the "only start xptIIM once all the components are visible" tenet that informed the original design of this restart mess. Darin and I would like to explore more why that change was made, and if there are other options that would avoid the need for this patch. (I do think that the patch moves the IIM in the right direction, and would be interested in taking some variant of it on the trunk.)
Comment 14•20 years ago
|
||
> (I do think that the patch moves the IIM in the right direction, and would be
> interested in taking some variant of it on the trunk.)
Absolutely! And, maybe for the trunk we can make it so that
AutoRegisterInterfaces rebuilds the path set automagically, thus avoiding the
need for the additional interface.
Comment 15•20 years ago
|
||
OK - this fixes all the bugs. This patch excludes extension components dirs from the components list while their "toBeInstalled" flag is set - i.e. while they're being installed or upgraded - so there will be no components loaded if we need to overwrite. Backs out change to the earlier bug (to nsAppRunner.cpp/nsXREDirProvider.cpp) about not being able to upgrade components.
Comment 16•20 years ago
|
||
Make XREDirProvider also not tell far-fetched adventure stories to the component manager about what components are available, now that we've guaranteed the component list in components.ini will be sane.
Attachment #158378 -
Attachment is obsolete: true
Assignee | ||
Comment 17•20 years ago
|
||
Comment on attachment 158379 [details] [diff] [review] patch I like it. Best so far, for sure. Darin will like it too, I bet.
Attachment #158379 -
Flags: superreview?(darin)
Attachment #158379 -
Flags: review+
Comment 18•20 years ago
|
||
Comment on attachment 158379 [details] [diff] [review] patch Looks good to me, let's get this done!
Attachment #158379 -
Flags: superreview?(darin) → superreview+
Comment 19•20 years ago
|
||
Ditto
Comment 20•20 years ago
|
||
Patch checked in. Shaver, you probably want to take your patch to a separate bug if you want to pursue it on the trunk.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Keywords: fixed-aviary1.0
Comment 21•20 years ago
|
||
Comment 22•20 years ago
|
||
Comment on attachment 158945 [details] [diff] [review] porting the changes in toolkit/xre to the trunk this doesn't completely port ben's patch, but i believe this bug doesn't exist on the trunk. i'm only doing this patch to ease merging aviary onto the trunk *a little bit*
Attachment #158945 -
Flags: review?(bsmedberg)
Updated•20 years ago
|
Attachment #158945 -
Flags: review?(bsmedberg) → review+
Comment 23•20 years ago
|
||
attachment 158945 [details] [diff] [review] has been checked in on the trunk
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•