Closed Bug 344132 Opened 14 years ago Closed 14 years ago

No user feedback when adding new search engine fails to load from nonexistent host

Categories

(Firefox :: Search, defect)

2.0 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 2 beta2

People

(Reporter: pamg.bugs, Assigned: Gavin)

References

Details

(Keywords: fixed1.8.1, regression)

Adding a new search engine whose description file is specified at a nonexistent host fails silently, with no user feedback, when using a new profile.
	javascript:window.sidebar.addSearchEngine('http://does-not-exist.mozilla.org/webster.src', 'http://addons.mozilla.org/search-engines-static/webster.gif', 'Webster', 'General')

When I run the above JS in my existing user profile, I get the expected error dialog and no assertions.  When I run it in a brand new profile, I get no error message, and two copies of the following assertion:

* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIHttpChannel.requestSucceeded]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: file:///Users/pamg/Development/mozilla/branch_source/mozilla/objects/dist/BonEchoDebug.app/Contents/MacOS/components/nsSearchService.js :: SRCH_loadStopR :: line 298"  data: no]

Compare this to the behavior when adding an engine from a valid host but a nonexistent file, i.e.
		javascript:window.sidebar.addSearchEngine('http://addons.mozilla.org/does-not-exist.src', 'http://addons.mozilla.org/search-engines-static/webster.gif', 'Nothing', 'General')

This second JS snippet also produces one copy of the above assertion when run in a new profile, and no assertions when run in an old profile.  It always produces the error dialog.

To summarize: 
Old profile, bad file: Error dialog, no assertions
Old profile, bad host: Error dialog, no assertions
New profile, bad file: Error dialog, one assertion
New profile, bad host: No error dialog, two assertions
A bit more investigation reveals that my test file had an error.  Please disregard  one assertion from each of the new-profile tests.  That is, the only case that produces any assertions is a new profile and a bad host: it sends one assertion as listed above, and does not show any error dialog.
Blocks: 335435
Ah, yeah I noticed this bug when working on the update system. We just need to check isSuccessCode(aStatus) before trying to get aRequest.requestSucceeded. I was planning to roll that into the patch for bug 327932.
(In reply to comment #2)
> We just need to check isSuccessCode(aStatus) before trying to get
> aRequest.requestSucceeded.

So why does the problem only show up for some profiles, in particular (in limited testing) empty ones?
(In reply to comment #3)
> So why does the problem only show up for some profiles, in particular (in
> limited testing) empty ones?

Hrm, I don't know why it wouldn't happen in your old profile. I can reproduce it consistently, with both old and new profiles.
(In reply to comment #4)
> Hrm, I don't know why it wouldn't happen in your old profile.

Further testing gave no more clues to that mystery.  But "this works when I don't think it should" is a pretty benign bug, so as long as the incorrect case is fixed I'm not going to worry about it much.
This should be fixed now that the search engine update patch (bug 327932) has landed on the trunk. Pam, can you verify?
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee: nobody → gavin.sharp
Yes, it's fixed.
Fixed1.8.1 by the landing of bug 327932.
Keywords: fixed1.8.1
You need to log in before you can comment on or make changes to this bug.