nsIStringBundle needs a method for checking whether a resource is missing

NEW
Assigned to

Status

()

--
enhancement
14 years ago
9 years ago

People

(Reporter: jshin1987, Assigned: jshin1987)

Tracking

({intl})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

14 years ago
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

14 years ago
This sounds like a good plan to me.  Remember to rev the UUID of the interface
when you make this change.
(Assignee)

Comment 2

14 years ago
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

14 years ago
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

14 years ago
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 ;-)
(Assignee)

Comment 5

14 years ago
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

14 years ago
Created attachment 186501 [details] [diff] [review]
add a DEBUG_smontagu around the warnings

Simon for more info, see comment 3
Attachment #186501 - Flags: superreview?(darin)
Attachment #186501 - Flags: review?(smontagu)

Comment 7

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

Updated

14 years ago
Attachment #186501 - Flags: superreview?(darin) → superreview-

Updated

14 years ago
Attachment #186501 - Attachment is obsolete: true
Attachment #186501 - Flags: review?(smontagu)

Comment 8

14 years ago
Created attachment 186537 [details] [diff] [review]
the fix with the right simon

Woops! How about this Simon :)

Simon, see comment 3 for details.
Attachment #186537 - Flags: superreview?(darin)
Attachment #186537 - Flags: review?(sfraser_bugs)

Updated

14 years ago
Attachment #186537 - Flags: superreview?(darin) → superreview+

Comment 9

14 years ago
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

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

Comment 11

14 years ago
Created attachment 186630 [details] [diff] [review]
[patch checked in]#if 0 instead of debug_sfraser

How about this one with the #if 0,  Simon?
Attachment #186537 - Attachment is obsolete: true
Attachment #186630 - Flags: superreview+
Attachment #186630 - Flags: review?(sfraser_bugs)

Comment 12

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

clearing an obsolete request
Attachment #186537 - Flags: review?(sfraser_bugs)

Comment 13

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

Sure
Attachment #186630 - Flags: review?(sfraser_bugs) → review+

Updated

14 years ago
Attachment #186630 - Flags: approval-aviary1.1a2?

Updated

14 years ago
Attachment #186630 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+

Updated

14 years ago
Attachment #186630 - Attachment description: #if 0 instead of debug_sfraser → [patch checked in]#if 0 instead of debug_sfraser
QA Contact: amyy → i18n
You need to log in before you can comment on or make changes to this bug.