Closed
Bug 347310
Opened 19 years ago
Closed 18 years ago
Implement notification callback to suppress extraneous microsummary update errors
Categories
(Firefox Graveyard :: Microsummaries, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 2
People
(Reporter: reed, Assigned: myk)
References
Details
(Whiteboard: [would take patch])
Attachments
(2 files, 2 obsolete files)
9.86 KB,
patch
|
Biesinger
:
review+
|
Details | Diff | Splinter Review |
1.15 KB,
patch
|
Details | Diff | Splinter Review |
My error console is being filled with "Error: [Exception... "'Component does not have requested interface' when calling method: [nsIInterfaceRequestor::getInterface]" nsresult: "0x80004002 (NS_NOINTERFACE)" location: "<unknown>" data: no]".
No idea what is causing it. This only started within the last couple of days. :/
gavin, I was told you would be good to ask about it. :)
Comment 1•19 years ago
|
||
Using any extensions?
Reporter | ||
Comment 2•19 years ago
|
||
Of course...
Adblock Filterset.G Updater 0.3.0.4
Adblock Plus 0.7.1.1
BugMeNot 1.3
DOM Inspector 1.8.1b1
FireBug 0.4
Forecastfox 0.9.3
IE Tab 1.0.9.5
Live HTTP Headers 0.12
Nightly Tester Tools 1.1
Talkback 2.0b1
ThinkVantage Password Manager 2.0
Tinderstatus 0.2.1
Update Channel Selector 1.0.1
User Agent Switcher 0.6.8
Web Developer 1.0.2
:P
Comment 3•19 years ago
|
||
There's not much to go on with just that information, that error could be thrown by any number of things. You should get venkman is leave it open and set to break on exceptions, then see what's throwing.
Reporter | ||
Comment 4•19 years ago
|
||
Thanks, myk!
I'm getting this error quite often, sadly. It's really annoying. :/
*** This bug has been marked as a duplicate of 253127 ***
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Updated•19 years ago
|
Status: RESOLVED → REOPENED
Component: General → Microsummaries
QA Contact: general → microsummaries
Resolution: DUPLICATE → ---
Target Milestone: --- → Firefox 2
Reporter | ||
Updated•19 years ago
|
Assignee: nobody → myk
Status: REOPENED → NEW
Reporter | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Flags: blocking-firefox2?
Summary: Error: [Exception... "'Component does not have requested interface' when calling method: [nsIInterfaceRequestor::getInterface]" nsresult: "0x80004002 (NS_NOINTERFACE)" location: "<unknown>" data: no] → Implement notification callback to suppress extraneous microsummary update errors
Reporter | ||
Comment 5•19 years ago
|
||
biesi mentioned a way to fix this to myk on IRC. Reopening with new summary. :)
OS: Windows XP → All
Hardware: PC → All
Updated•19 years ago
|
Flags: blocking-firefox2? → blocking-firefox2-
Whiteboard: [would take patch]
Comment 6•19 years ago
|
||
Discussed with myk, only an annoyance if chrome errors are shown, it can be fixed with low risk, but has no user benefit.
Assignee | ||
Comment 8•19 years ago
|
||
It looks like nsIPrompt is the interface being requested in all the cases I've investigated. Here's a minimal fix that makes MicrosummaryResource claim to implement nsIPrompt (which it partly does anyway, since it implements nsIAuthPrompt, which has prompt, promptPassword, and promptUsernameAndPassword methods just like nsIPrompt does).
But what if a caller tried to call one of the other methods on nsIPrompt? Presumably XPConnect would throw an error, but is that any worse than the current situation? Or, put another way, does this patch make things better without making anything worse, or should I be implementing the rest of nsIPrompt in MicrosummaryResource?
Attachment #252973 -
Flags: review?
Assignee | ||
Updated•19 years ago
|
Attachment #252973 -
Flags: review? → review?(cbiesinger)
Comment 9•19 years ago
|
||
(In reply to comment #8)
> implement nsIPrompt (which it partly does anyway, since it implements
> nsIAuthPrompt, which has prompt, promptPassword, and promptUsernameAndPassword
> methods just like nsIPrompt does).
Not really, their signatures are different...
> But what if a caller tried to call one of the other methods on nsIPrompt?
> Presumably XPConnect would throw an error, but is that any worse than the
> current situation? Or, put another way, does this patch make things better
> without making anything worse, or should I be implementing the rest of
> nsIPrompt in MicrosummaryResource?
Yes, XPConnect throws an exception in that case, and also prints a console message IIRC. But given the different signatures, I'm not sure I like this patch...
Updated•19 years ago
|
Attachment #252973 -
Flags: review?(cbiesinger) → review-
Assignee | ||
Comment 10•18 years ago
|
||
(In reply to comment #9)
> Yes, XPConnect throws an exception in that case, and also prints a console
> message IIRC. But given the different signatures, I'm not sure I like this
> patch...
Ok, here's a version that implements nsIAuthPrompt and nsIPrompt in separate objects to avoid signature collision.
It's unclear when nsIPrompt methods actually get called (they don't get called in my testcase, even though the interface gets requested), but I abort the load if the interface requester calls nsIPrompt.promptPassword or nsIPrompt.promptUsernameAndPassword.
Otherwise I throw NS_ERROR_NOT_IMPLEMENTED but don't abort the load, assuming that the load is complete (enough) to be considered successful, despite the load's inability to alert() the user to something or confirm() something with them. Perhaps that's an faulty assumption.
Other fixes in this patch:
1. added a Cr shortcut (like the existing Cc and Ci) for Components.results;
2. added three nsIAuthPrompt constants to its implementation for completeness;
3. updated comment about which notification callback interfaces we implement,
since I have been informed that it's not actually necessary to be
conservative about which interfaces we implement/override, as XMLHTTPRequest
ensures that it gets notified about the ones it cares about (namely
nsIProgressEventSink).
Steps to Reproduce:
1. Make sure chome errors are enabled (javascript.options.showInConsole).
2. Install the "Yahoo! Finance Stock Quote w/Change" microsummary generator at:
http://userstyles.org/livetitle
3. Bookmark a Yahoo! Finance Stock Quote page and use its live title, f.e.:
http://finance.yahoo.com/q?s=twx
4. Close the page to make sure the microsummary service downloads it anew
in the next step.
5. Context-click the bookmark and select "Reload Live Title" from the menu.
Without the patch, the following error should appear in the error console:
Error: [Exception... "'Component does not have requested interface' when calling method: [nsIInterfaceRequestor::getInterface]" nsresult: "0x80004002 (NS_NOINTERFACE)" location: "<unknown>" data: no]
With the patch, the error should not appear.
Attachment #252973 -
Attachment is obsolete: true
Attachment #254369 -
Flags: review?(cbiesinger)
Assignee | ||
Comment 11•18 years ago
|
||
One more fix in that patch:
4. Renamed _httpAuthFailed to _authFailed, since it's not clear that nsIPrompt
authentication methods are specifically for HTTP authentication.
Comment 13•18 years ago
|
||
HTTP doesn't use the password functions from nsIPrompt. It only uses the confirm function(s) in a few cases. Actually I don't think any necko protocol uses the password functions.
Comment 14•18 years ago
|
||
Comment on attachment 254369 [details] [diff] [review]
patch v2: the non-minimal fix
// redirects us to a "you're not authorized" page), so we have to set a flag
// to let the load handler know to treat the load as a failure.
well the other way would be to check the requestSucceeded property of the channel (via nsIHttpChannel)... hm, or as this is an XMLHttpRequest, check its .status whether its a 2xx one. that way, you'd also not treat 404 pages as success.
// XXX If necko always requests nsIAuthPromptProvider before requesting
// nsIAuthPrompt, then we probably only have to implement the provider.
HTTP does do that. but not all channels use nsIAuthPromptProvider at all.
+ QueryInterface: function(iid) {
+ if (iid != Ci.nsIAuthPrompt)
+ throw Cr.NS_ERROR_NO_INTERFACE;
you need to allow nsISupports at least.
+ SAVE_PASSWORD_NEVER: Ci.nsIAuthPrompt.SAVE_PASSWORD_NEVER,
+ SAVE_PASSWORD_FOR_SESSION: Ci.nsIAuthPrompt.SAVE_PASSWORD_FOR_SESSION,
+ SAVE_PASSWORD_PERMANENTLY: Ci.nsIAuthPrompt.SAVE_PASSWORD_PERMANENTLY,
what's the point of these? (also for the nsIPrompt ones)
+ QueryInterface: function(iid) {
+ if (iid != Ci.nsIPrompt)
+ throw Cr.NS_ERROR_NO_INTERFACE;
again, need nsISupports
Attachment #254369 -
Flags: review?(cbiesinger) → review+
Assignee | ||
Comment 15•18 years ago
|
||
(In reply to comment #14)
> well the other way would be to check the requestSucceeded property of the
> channel (via nsIHttpChannel)... hm, or as this is an XMLHttpRequest, check its
> .status whether its a 2xx one. that way, you'd also not treat 404 pages as
> success.
Hmm, that's a very good idea, and since we only support the http, https, and file schemes, and the local file channel doesn't do auth (as far as I know), we could just drop _authFailed and look at the requestSucceeded property or the status code.
But if we ever want to support additional schemes that do authentication and don't provide requestSucceeded or a status code (ftp?), then we'll probably need to reimplement an _authFailed-like workaround.
In any case, making this kind of change seems like a separate bug, given that we are already doing _authFailed, and all this patch is doing is using that existing mechanism for an additional set of authentication prompts. I've filed it as bug 370242.
> // XXX If necko always requests nsIAuthPromptProvider before requesting
> // nsIAuthPrompt, then we probably only have to implement the provider.
>
> HTTP does do that. but not all channels use nsIAuthPromptProvider at all.
Ok, I've updated this comment to read:
// HTTP always requests nsIAuthPromptProvider first, so it never needs
// nsIAuthPrompt, but not all channels use nsIAuthPromptProvider, so we
// implement nsIAuthPrompt too.
> + QueryInterface: function(iid) {
> + if (iid != Ci.nsIAuthPrompt)
> + throw Cr.NS_ERROR_NO_INTERFACE;
>
>
> you need to allow nsISupports at least.
Ok, I've added nsISupports (and switched to using the same implementation of QueryInterface that I use elsewhere in that file).
> + SAVE_PASSWORD_NEVER: Ci.nsIAuthPrompt.SAVE_PASSWORD_NEVER,
> + SAVE_PASSWORD_FOR_SESSION: Ci.nsIAuthPrompt.SAVE_PASSWORD_FOR_SESSION,
> + SAVE_PASSWORD_PERMANENTLY: Ci.nsIAuthPrompt.SAVE_PASSWORD_PERMANENTLY,
>
> what's the point of these? (also for the nsIPrompt ones)
I was under the mistaken impression that I needed to define them for my component to fully implement the interface. Vlad corrected me on IRC, noting that "the constants are put in the xpt for JS use, and are available as anonymous enums in C++ code". I've removed them.
> + QueryInterface: function(iid) {
> + if (iid != Ci.nsIPrompt)
> + throw Cr.NS_ERROR_NO_INTERFACE;
>
> again, need nsISupports
Ok, I've added nsISupports (and switched implementation) here too.
Attachment #254369 -
Attachment is obsolete: true
Attachment #254905 -
Flags: review?(cbiesinger)
Updated•18 years ago
|
Attachment #254905 -
Flags: review?(cbiesinger) → review+
Assignee | ||
Comment 16•18 years ago
|
||
Checked in to the trunk:
Checking in browser/components/microsummaries/src/nsMicrosummaryService.js;
/cvsroot/mozilla/browser/components/microsummaries/src/nsMicrosummaryService.js,v <-- nsMicrosummaryService.js
new revision: 1.54; previous revision: 1.53
done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 17•18 years ago
|
||
The patch v3 broke bookmarks due to a syntax error. The error is fixed by this patch, which I've checked in to the trunk:
Checking in browser/components/microsummaries/src/nsMicrosummaryService.js;
/cvsroot/mozilla/browser/components/microsummaries/src/nsMicrosummaryService.js,v <-- nsMicrosummaryService.js
new revision: 1.55; previous revision: 1.54
done
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•