Closed Bug 372076 Opened 18 years ago Closed 18 years ago

IConvAdaptor::SetOutputErrorBehavior should handle nsIUnicodeEncoder::kOnError_Signal

Categories

(Core :: Internationalization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: romaxa, Assigned: sciguyryan)

Details

Attachments

(1 file)

IConvAdaptor::SetOutputErrorBehavior should also handle nsIUnicodeEncoder::kOnError_Signal: and set mReplaceOnError = PR_FALSE; in this case.
Attached patch patch v1Splinter Review
Patch v1 * As suggested by Romaxa. Simon, if this patch is correct do we need a sr here? (I'm not sure on the intl sr policy).
Assignee: smontagu → sciguyryan
Status: NEW → ASSIGNED
Attachment #256809 - Flags: review?(smontagu)
Comment on attachment 256809 [details] [diff] [review] patch v1 intl policy is that code changes need sr. For changes that only effect properties files or charset conversion tables, r= from one of the module owners is sufficient.
Attachment #256809 - Flags: review?(smontagu) → review+
(In reply to comment #2) > (From update of attachment 256809 [details] [diff] [review]) > intl policy is that code changes need sr. For changes that only effect > properties files or charset conversion tables, r= from one of the module owners > is sufficient. > I'll remember that thanks. Can you suggest a sr for this case?
On second thoughts, intl/uconv/native isn't part of the default build, so I don't think you need sr. Go ahead and check in.
OS: Linux → All
Hardware: PC → All
Whiteboard: [checkin needed]
Checked in: mozilla/intl/uconv/native/nsNativeUConvService.cpp 1.7 Does this need / can this have a unit test?
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
I don't see why not, in theory at least, though as far as I know none of the tinderboxen build with native uconv right now.
Flags: in-testsuite?
this checkin is causing a libjar unit test to fail, so first we should sort that out.
(In reply to comment #7) > this checkin is causing a libjar unit test to fail nevermind me, it was bug 370103
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: