Closed Bug 1201468 Opened 4 years ago Closed 4 years ago

Conversion function from ICU UErrorCode to nsresult

Categories

(Core :: Internationalization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox43 --- affected
firefox44 --- fixed

People

(Reporter: smontagu, Assigned: smontagu)

Details

Attachments

(1 file, 1 obsolete file)

As we move ahead with using ICU API to replace old i18n functionality, it will be useful to have a utility function to map ICU's UErrorCode to nsresult return values.
Attached patch Patch (obsolete) — Splinter Review
Attachment #8656500 - Flags: review?(jfkthame)
Comment on attachment 8656500 [details] [diff] [review]
Patch

Review of attachment 8656500 [details] [diff] [review]:
-----------------------------------------------------------------

::: intl/unicharutil/util/ICUUtils.cpp
@@ +179,5 @@
> +ICUUtils::UErrorToNsResult(const UErrorCode aErrorCode)
> +{
> +  switch(aErrorCode) {
> +    case U_ZERO_ERROR:
> +      return NS_OK;

I'm not sure this is right. UErrorCode can contain various warning values that are not actual errors (and so we probably don't want to map them to NS_ERROR_FAILURE), but will make it fail a direct equality comparison with U_ZERO_ERROR.

What we probably want to do is something like

  if (U_SUCCESS(aErrorCode)) {
    return NS_OK;
  }

before getting into the switch() to map specific error codes.
Attached patch Patch v.2Splinter Review
Yes, agreed.
Attachment #8656500 - Attachment is obsolete: true
Attachment #8656500 - Flags: review?(jfkthame)
Attachment #8656527 - Flags: review?(jfkthame)
Comment on attachment 8656527 [details] [diff] [review]
Patch v.2

Review of attachment 8656527 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine, thanks.
Attachment #8656527 - Flags: review?(jfkthame) → review+
https://hg.mozilla.org/mozilla-central/rev/05f45d36851d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.