Robert, would you like to mentor this bug?

(In reply to Philip Chee from comment #1) > Robert, would you like to mentor this bug? Not really, I rarely have any time to care about any SeaMonkey topics, and people are leaving me enough work with untested changes in modules I own that all SeaMonkey time I have this year is probably booked out already right now.

Comment on attachment 647887 [details] [diff] [review] patch v0 Looks good so far, but you also have to change: http://mxr.mozilla.org/comm-central/source/suite/mailnews/searchBar.js#19

In reply to comment #4: Ah, OK, I'd missed that. I ought to have run an mxr search for "searchFailureMessage" (now I did, and nothing else came up).

oops, wrong indent in "else" branch of if

Comment on attachment 648991 [details] [diff] [review] patch v1.1 >+++ b/suite/mailnews/searchBar.js > function SetQSStatusText(aNumHits) > { > var statusMsg; > // if there are no hits, it means no matches were found in the search. >+ if (aNumHits == 0) { >+ statusMsg = gSearchBundle.getString("noMatchesFound"); >+ } else { >+ statusMsg = PluralForm.get(aNumHits, >+ gSearchBundle.getString("matchesFound"); Missing ) after argument list. r=me with that fixed.

Duh. I really should have pasted the change by mouse instead of by eyeball.

oops, it's a mailnews patch.

Comment on attachment 649085 [details] [diff] [review] patch v1.2 r=IanN Just one nit: >+ if (aNumHits == 0) { >+ statusMsg = gSearchBundle.getString("noMatchesFound"); >+ } else { >+ statusMsg = PluralForm.get(aNumHits, >+ gSearchBundle.getString("matchesFound")); >+ statusMsg = statusMsg.replace("#1", aNumHits); > } Braces should go onto their own line, see <https://wiki.mozilla.org/SeaMonkey:MailNews:CodingStyle#Default_bracing_style_is_.22curly_braces_go_on_their_own_line.22>. moa=me with that fixed throughout the whole patch.

Ah, OK. …and I also saw the next section, "braces on both sides of an else, or on neither", so the braces around the single-statement "true" paths must stay. Hg qrefresh removed and added the else rather than the { but what can I do? Thats how it is. :-/

Checking this fix correctly requires testing a localized version in a language with more than two "plural forms". The UI languages currently built every night for Sm trunk are: be, de, en-US, es-AR, hu, it, gl, lt, nl, ru, sk, uk, zh-CN, zh-TW but of course the localizers will need some time to add the new strings, which are: - "No matches found" (existed already, but the string name changed) - "#n match(es) found" (new localizable string, depending on the value of the integer n >= 1). The second string has in practice two values (as shown) for English; in other languages it may be fewer (Chinese=1), the same (Western European), or more.

Comment on attachment 649140 [details] [diff] [review] patch v1.3 r=IanN moa=Mnyromyr Pike: How do you think this fix should best be VERIFIED?

Comment on attachment 649140 [details] [diff] [review] patch v1.3 r=IanN moa=Mnyromyr Review of attachment 649140 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, no good idea. Not sure if you could hook up different plural rules by executing a line like http://mxr.mozilla.org/mozilla-central/source/intl/locale/src/PluralForm.jsm#101 with an explicit number for the plural form and test for that.