Closed Bug 285211 Opened 20 years ago Closed 18 years ago

xpcom component registration picking up the wrong copies of components (e.g,., after viewing about:plugins)

Categories

(Camino Graveyard :: General, defect, P2)

PowerPC
macOS

Tracking

(Not tracked)

RESOLVED FIXED
Camino1.5

People

(Reporter: alqahira, Assigned: stuart.morgan+bugzilla)

Details

(Keywords: fixed1.8.0.13, fixed1.8.1.4)

Attachments

(3 files, 3 obsolete files)

"Warn on enter a secure site" has apparently been permanently disabled in Camino (setting the security.warn_entering_secure* prefs has no effect); however, viewing about:plugins re-enables the warning (for that browser session). This is presumably a regression from the first appearance of a working about:plugins, but I first noticed it in 2005030608 (v0.8+). At the moment that also makes Camino subject to bug 284357.
Using 2005032408 (v0.8+): Same thing happens. I got the same warning.
It also re-enables the "warn on submitting insecure form" dialogue. I'm guessing at this point that there are other dialogues that have been hard-coded off in Camino that viewing about:plugins is going to re-enable. Bizarre.
Summary: viewing about:plugins reenables "warn on entering secure site" → viewing about:plugins reenables "warn on entering secure site", "warn submitting insecure form"
This appears to be WFM these days.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → WORKSFORME
this just happened to me with 1.0b2.
Hrm, I just tried 1.0b2, too, and still WFM.
Reopening; Mike isn't crazy, but maybe I was before. Visiting about:plugins *does* re-enable "warn on entering secure site" and "warn on submitting an insecure form" in 2006012904 (v1.0b2+) ("Warn on submitting an insecure form from a secure website" seems to be on permanantly, unrelated to this regression)
Status: RESOLVED → REOPENED
Keywords: regression
Resolution: WORKSFORME → ---
I was so crazy...this was broken the day in July I claimed it was WFM. In fact, it was broken back as far as 0.8.4...we just didn't notice since about:plugins displayed nothing and wasn't a menu item until Mar 2005. Setting security.warn_entering_secure to false seems to prevent that behavior, so for that pref, adding it to all-camino.js might be a viable quick work-around. Setting security.warn_submit_insecure to false also seems to prevent that sheet from displaying. Still, visiting about:plugins manages to distort/change the "warn on submitting an insecure form from a secure website" sheet.
Keywords: regression
Weird; does it happen in FF too? Loading about:plugins causes XPCOM to re-register components, so maybe some component is resetting those defaults at that time?
I can't reproduce this. Does it depend on what plugins you have installed?
i don't have anything weird installed. - flash - java - shockwave - WMP - quicktime - adobe pdf viewer - drm plugin (part of wmp, it seems)
No, it doesn't happen in Fx, but it doesn't disable those dialogues in code, either (or at least that looks like what we're doing here: http://landfill.mozilla.org/mxr-test/mozilla/source/camino/src/browser/SecurityDialogs.mm#902 see also lines 916 and 962). Fx displays *all* of the security dialogues on first encounter (and the "always warn me" check box defaults to unchecked in every single one, not just the stupid ones!), and it honors the prefs thereafter. There are additional prefs to cover the checkboxes (a .show_once partner to each warn.foo). Fx also doesn't seem to have a custom "secure site->insecure form" dialogue; it has the same text as the second image I posted above, regardless of whether Fx has visited about:plugins or not.
Put this patch in MainController: Index: MainController.mm =================================================================== RCS file: /cvsroot/mozilla/camino/src/application/MainController.mm,v retrieving revision 1.170 diff -u -r1.170 MainController.mm --- MainController.mm 29 Jan 2006 04:56:29 -0000 1.170 +++ MainController.mm 30 Jan 2006 16:32:41 -0000 @@ -261,6 +261,12 @@ // listen for the Show Certificates notification (which is send from the Security prefs panel) [notificationCenter addObserver:self selector:@selector(showCertificatesNotification:) name:@"ShowCertificatesNotification" object:nil]; +// testing + [notificationCenter addObserver:self selector:@selector(prefChanged:) name:kPrefChangedNotificationName object:nil]; + [[PreferenceManager sharedInstance] addObserver:self forPref:"security.warn_entering_secure"]; +// testing + + [self setupStartpage]; // Initialize offline mode. @@ -296,6 +302,12 @@ [self performSelector:@selector(checkDefaultBrowser) withObject:nil afterDelay:2.0f]; } + +- (void)prefChanged:(NSNotification*)inNote +{ + NSLog(@"Pref changed"); +} + - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication *)sender { ProgressDlgController* progressWindowController = [ProgressDlgController existingSharedDownloadController]; and break in the debugger in the prefChanged:. Then go to about:plugins and see who's setting that.
I can repro with just the JEP and the DefaultPlugin (i.e., just what's shipped with the app).
This has the same root cause as bug 278928. The issue is that loading about:plugins causes XPCOM to do autoregistration. This allows dylibs to override our internal components that we hand-registered with NS_NewGenericFactory()/RegisterFactory(). So after loading about:plugins, you're getting NSS's NS_SECURITYWARNINGDIALOGS implementation, and not Camino's. dougt: is there a way we can get notified when autoreg happens, and re-register our internal components last?
We probably need to add an observer for NS_XPCOM_AUTOREGISTRATION_OBSERVER_ID, "end" and re-register our components there.
Assignee: mikepinkerton → nobody
Status: REOPENED → NEW
QA Contact: general
Target Milestone: --- → Camino2.0
I think the underlying issue (xpcom component registration picking up the wrong copies of components) is serious enough to warrant fixing before 2.0. As it is now, Camino is very susceptible to changes in core components. I suggest an approach to fix this in comment 17.
Severity: minor → major
Priority: -- → P2
Summary: viewing about:plugins reenables "warn on entering secure site", "warn submitting insecure form" → xpcom component registration picking up the wrong copies of components (e.g,., after viewing about:plugins)
Target Milestone: Camino2.0 → Camino1.1
#17 seams like the easiest thing to do. all embedding applications are going to have this problem if they override components: http://lxr.mozilla.org/mozilla1.8/ident?i=RegisterFactory
Make a simple .xul window <?xml version="1.0"?> <?xml-stylesheet href="chrome://global/skin/" type="text/css"?> <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" width="640" height="480"> <button flex="1" label="click" oncommand="alert(Components.interfaces.nsIExeDataExtractor)"/> </window> The nsIExeDataExtractor doesn't get through to JS. Clicking the button gives undefined, works ok in ff though. Maybe this is associated with this bug. Tested on two PCs happened when upgrading my app, seemed to be getting the older interfaces but not the new ones.
(In reply to comment #20) > Maybe this is associated with this bug. It isn't.
Per comment 18, this should block 1.1. Who's up for taking it?
Flags: camino1.1+
Stuart, is this something you can fix (comment 17 is the desired approach)?
Flags: camino1.1a1?
Attachment #228791 - Attachment description: Testcase idl file. Compile and import in simple xul app → Testcase idl file. Compile and import in simple xul app (not this bug)
Attachment #228791 - Attachment is obsolete: true
Flags: camino1.1a1? → camino1.1a1-
Assignee: nobody → stuart.morgan
Either I'm missing something, or the docs at http://www.mozilla.org/projects/embedding/observer-topics.html are lying. For xpcom-autoregistration it says "You can register for this event by using the nsIObserver Service", and for xpcom-shutdown it says "You can register for this event either by using the nsIObserver Service or by adding a category entry under the category of the topic name". Yet if I make an nsIObserver subclass and register it with nsIObserverService, it works fine for xpcom-shutdown but never gets called for xpcom-autoregistration (and I'm manually triggering AutoRegister and veryifying it's being called in the debugger). If I look at NS_ShutdownXPCOM I see that it calls NotifyObservers, whereas AutoRegister instead calls a function which enumerates categories and calls Observe on any that conform to nsIObserver. I'm certainly not familiar with this code, but that sounds completely backwards from what the docs say. Could someone familiar with this weigh in on what's going on here?
bsmedberg, josh suggested you'd be a good person to comment on comment 24, and specifically what the right way to listen for xpcom autoregistration would be.
I don't know. registerFactory is obviously a terrible hack, but it seems that you'd have the same problem if you were using the recommended technique for registering static components (passing a static component list to NS_InitXPCOM3).
Should this block beta? cl
Flags: camino1.1b1?
I've started experimenting with doing this as a category entry, but it's much more involved and I'm learning as I go, so I haven't figured out if it works yet. If anyone could point me to existing code that listens for auto-registration and actually works, it would help a lot.
I was able to experience this bug again in yesterday's branch nightly after playing with the disable/enable plugins pref and choosing Installed Plug-Ins a few times.... (Hmm, I thought there was a comment about us not seeing it and not being able to repro when Stuart started working on this...maybe he just mentioned it on irc or at a meeting?)
Since I still don't know why code that seems like it should work doesn't, I don't think this should hold b1. I'll keep trying for 1.1 final though.
Minusing for b1 per comment 30.
Flags: camino1.1b1? → camino1.1b1-
Attached patch fix (obsolete) (deleted) — Splinter Review
The category method of listening works fine, so I guess the docs just lie. Simon, would you be willing to look this over? If not, feel free to just reassign the review to a generic r?
Attachment #259552 - Flags: review?(sfraser_bugs)
you don't need NS_INIT_ISUPPORTS() anymore
Comment on attachment 259552 [details] [diff] [review] fix This all looks good. My only question is whether you could simply add your sAutoregistrationListenerComponentInfo to the list of components we already have in AppComponents.mm, and have it registered by the existing CHBrowserService::RegisterAppComponents() code.
Attachment #259552 - Flags: review?(sfraser_bugs) → review+
I thought about that, but I like to pretend sometimes that this will one day be the framework it was originally intended to be, in which case it would be nice to have the behavior built-in rather than all its clients having to do the right thing.
Attached patch v2 (obsolete) (deleted) — Splinter Review
Removes NS_INIT_ISUPPORTS
Attachment #259611 - Flags: superreview?(mikepinkerton)
Comment on attachment 259611 [details] [diff] [review] v2 + // Set it to listen for XPCOM autoregistration. + nsCOMPtr<nsICategoryManager> cm = do_GetService(NS_CATEGORYMANAGER_CONTRACTID, &rv); + if (NS_SUCCEEDED(rv)) { usually it's ok to use the non-rv version of this call and just check if the service is NULL. Since you really don't care about *what* failed, just makes it a little cleaner. sr=pink
Attachment #259611 - Flags: superreview?(mikepinkerton) → superreview+
That's kind of embarrassing, given that I made the same argument in one of my recent patches and have been thinking about cleaning up our existing do_GetService calls ;)
Checked in with do_GetService change on trunk and MOZILLA_1_8_BRANCH.
Status: NEW → RESOLVED
Closed: 19 years ago18 years ago
Keywords: fixed1.8.1.4
Resolution: --- → FIXED
Should we look at taking this for 1.0.5?
Flags: camino1.0.5?
Approving for 1.0.5.
Flags: camino1.0.5? → camino1.0.5+
Whiteboard: [needs checkin 1.8.0]
Attached patch v3 (deleted) — Splinter Review
As checked in on MOZILLA_1_8_0_BRANCH
Attachment #259552 - Attachment is obsolete: true
Attachment #259611 - Attachment is obsolete: true
Whiteboard: [needs checkin 1.8.0]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: