293023 - nsIStringBundle needs a method for checking whether a resource is missing

Status: RESOLVED WONTFIX

(Core :: Internationalization, enhancement)

Description

[Jungshik Shin]

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.

Comment 1

[Darin Fisher]

This sounds like a good plan to me.  Remember to rev the UUID of the interface
when you make this change.

Comment 2

[Jungshik Shin]

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

Comment 3

[Scott MacGregor]

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....

Comment 4

[Darin Fisher]

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 ;-)

Comment 5

[Jungshik Shin]

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.

Comment 6

[Scott MacGregor]

Attachment: add a DEBUG_smontagu around the warnings

Simon for more info, see comment 3

Comment 7

[Darin Fisher]

Simon Montagu or Simon Fraser?  comment #0 says sfraser ;-)

Comment 8

[Scott MacGregor]

Attachment: the fix with the right simon

Woops! How about this Simon :)

Simon, see comment 3 for details.

Comment 9

[Simon Fraser [no longer active]]

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).

Comment 10

[Darin Fisher]

Perhaps a better solution would be to use NSPR logging, but whatever works ;-)

Comment 11

[Scott MacGregor]

Attachment: [patch checked in]#if 0 instead of debug_sfraser

How about this one with the #if 0,  Simon?

Comment 12

[Scott MacGregor]

Comment on attachment 186537 [details] [diff] [review]
the fix with the right simon

clearing an obsolete request

Comment 13

[Simon Fraser [no longer active]]

Comment on attachment 186630 [details] [diff] [review]
[patch checked in]#if 0 instead of debug_sfraser

Sure

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

we're not going to invest in further improving StringBundle.