To reproduce: 1. Fetch SeaMonkey trunk build 2. Select a folder from an IMAP account, may also happen with POP3, dunno 2. Try to select Tools->Search Messages... Results: "Search Messages..." is grayed out and cannot be selected. This is probably a regression, I think this might have to do with Bug 422814. It changed the function nsImapIncomingServer::GetPrefForServerAttribut under http://hg.mozilla.org/comm-central/annotate/e4f4569d451a/mailnews/imap/src/nsImapIncomingServer.cpp#l26. This function is called from nsImapIncomingServer::GetCanSearchMessages (http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapIncomingServer.cpp#2465) which is called by http://hg.mozilla.org/comm-central/annotate/e4f4569d451a/mailnews/base/resources/content/mail3PaneWindowCommands.js#l865. The old function code under nsImapIncomingServer::GetPrefForServerAttribut did not make that much sense (it did not use the prefSuffix param, see the patch/diff), but with the new code I get this bug here.

Confirmed with a TB build, so MailNews Core.

I think the old code didn't touch the out variable if the pref didn't exist, and the callers all set it to true, but the new code sets it to false if the pref doesn't exist.

file|compact folders is toast too

I meant http://hg.mozilla.org/comm-central/annotate/e4f4569d451a/mailnews/imap/src/nsImapIncomingServer.cpp#l2691 in the first comment as first link.

What this patch does is to make GetPrefForServerAttribute act as it did before, i.e., don't change the value if the preference doesn't exist, unlike GetBoolValue.

Comment on attachment 338413 [details] [diff] [review] Fix [Checkin: Comment 8] ah, thx, Joshua.

Checked in as changeset b63a39af19cd.

Still broken for me with a SeaMonkey trunk build.

GetBoolValue always returns NS_OK, so the NS_FAILED check cannot work. Maybe a clean fix would be better (no longer use the prefValue arg as in/out param).

does this also break other things in some way? message counts, filtering

can I kill comment 11? :) ignore it - started with wrong profile.

Breaks basic functions of SeaMonkey MailNews (also see Bug 454936), so requesting blocking?.

Still broken with thunderbird also. The first time it might be non-grayed out, but it doesn't bring up the search dialog. File | Compact and the Compact Button are always disabled for IMAP too.

So, I had the idea to add a parameter with the default value to GetBoolValue. That parameter is assigned to the return value when no pref was found. Comments welcome.

Comment on attachment 338554 [details] [diff] [review] Idea for a patch >- if (NS_FAILED(mPrefBranch->GetBoolPref(prefname, val))) >- mDefPrefBranch->GetBoolPref(prefname, val); >+ if (NS_FAILED(mPrefBranch->GetBoolPref(aPrefname, aVal))) >+ if (NS_FAILED(mDefPrefBranch->GetBoolPref(aPrefname, aVal))) >+ *aVal = aDefaultValue; Looks like I forgot { ... } after the first if-clause.

(In reply to comment #15) > Created an attachment (id=338554) [details] > Idea for a patch > > So, I had the idea to add a parameter with the default value to GetBoolValue. > That parameter is assigned to the return value when no pref was found. Comments > welcome. This seems a bit like overkill, when we could just replicate the appropraite actions on the function. It also means we have an inconsistent set of APIs for getting the different types of prefs. Have you also checked for javascript usages? I guess making that default paramenter [optional] would save changing those as well.

painful to use latest trunk

This is basically the GetBoolValue function implemented in the GetPrefForServerAt function, just with the input parameter as default return value when getting the prefs failed.

Comment on attachment 338632 [details] [diff] [review] Another patch [Checkin: See comment 22] Please add a comment at the start of the function stating that the caller must initialise prefValue for the default value. Note that we're accepting this in this format because those prefs should disappear because they are related to aol setups that weren't supported, and we believe they now are.

(In reply to comment #20) > Note that we're accepting this in this format because those prefs should > disappear because they are related to aol setups that weren't supported, and we > believe they now are. I filed bug 455326.

Pushed to c-c, changeset 713c34b1a232.

This FIXED bug is flagged with in‑testsuite? It would be great if assignee or someone else can clear the flag if a test is not appropriate. And if appropriate, create a test and plus the flag to finish off the bug.