sfraser added a warning when a string resource is missing in March. http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsStringBundle.cpp&branch=&root=/cvsroot&subdir=/mozilla/intl/strres/src&command=DIFF_FRAMESET&rev1=1.144&rev2=1.145 This change resulted in a lot of spurrious warnings (charset menu, mailnews, charset manager) because in charsetData.properties and charsetaliases.properties, only a subset of charsets have properties for 'notForOutgoing' , 'notForBrowser', 'alias' and so forth. Needless to say, this doesn't affect optimized builds at all, but it kinda get in the way of working with a debug build. I believe the warning was added for a reason so that I wouldn't suggest getting rid of them. Instead, I think we need to add |Has(wstring name, bool result)| to nsIStringBundle. Its implementation relies on nsIPersistentProperties and |Has| is not yet implemented there.
This sounds like a good plan to me. Remember to rev the UUID of the interface when you make this change.
Off-line, Scott identified another place where this would be useful. When sending an email in HTML, every character in the message body is checked against the html entity list, which generates almost as many warnings as characters in the message body (because most characters don't have entities associated with them). http://lxr.mozilla.org/seamonkey/source/intl/unicharutil/src/nsEntityConverter.cpp#257
in the interim of a real patch, can we ifdef DEBUG the warning for just Simon? or comment out the warning? It's really painful to use debug builds because the console gets so clogged up dumping thousands of warning messages whenever I try to send a message or save it as a draft. I try to disable the warning but with so many trees I never disable it everywhere....
Yeah, I agree that the warning is very annoying. Scott: why don't you post a patch for the #ifdef'ing? I'll review. Send mail to Simon to let him know what's up. Maybe he'll volunteer a better patch ;-)
I wrote a patch that does what I wrote in comment #0 over a month ago, but I had a very peculiar problem (I forgot exactly what...) for which I failed to figure out the cause, which is why I haven't uploaded the patch here yet. Given that, '#ifdef'ing seems to be a good idea for now.
Created attachment 186501 [details] [diff] [review] add a DEBUG_smontagu around the warnings Simon for more info, see comment 3
Simon Montagu or Simon Fraser? comment #0 says sfraser ;-)
Created attachment 186537 [details] [diff] [review] the fix with the right simon Woops! How about this Simon :) Simon, see comment 3 for details.
I didn't add the warning. I just changed it from an NS_WARNING to something that actually tells you which string is missing. Having said that, I'm fine with just putting #if 0 around the warning (don't bother with DEBUG_smfr).
Perhaps a better solution would be to use NSPR logging, but whatever works ;-)
Created attachment 186630 [details] [diff] [review] [patch checked in]#if 0 instead of debug_sfraser How about this one with the #if 0, Simon?
Comment on attachment 186537 [details] [diff] [review] the fix with the right simon clearing an obsolete request
Comment on attachment 186630 [details] [diff] [review] [patch checked in]#if 0 instead of debug_sfraser Sure
Attachment #186630 - Flags: review?(sfraser_bugs) → review+
Attachment #186630 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Attachment #186630 - Attachment description: #if 0 instead of debug_sfraser → [patch checked in]#if 0 instead of debug_sfraser
You need to log in before you can comment on or make changes to this bug.