reduce narrow windows API calls in uconv

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: blassey, Assigned: crowderbt)

Tracking

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

No description provided.
Attachment #309235 - Flags: review?(smontagu)
Comment on attachment 309235 [details] [diff] [review]
directy from "other" patch on bug 418703

>-  if (GetLocaleInfo(localeAsLCID, LOCALE_IDEFAULTANSICODEPAGE, acp_name, sizeof(acp_name))==0) { 
>+  if (GetLocaleInfoW(localeAsLCID, LOCALE_IDEFAULTANSICODEPAGE, acp_name, sizeof(acp_name))==0) { 
>     return NS_ERROR_FAILURE; 
>   }
>   nsAutoString acp_key(NS_LITERAL_STRING("acp."));
>-  acp_key.AppendWithConversion(acp_name);
>+  acp_key.Append(acp_name);

So, my first reaction to this was that it's moving in the wrong direction and I would rather see a patch that keeps the narrow API and makes acp_key a nsCAutoString. I see that the question "why are we doing this?" was already asked and answered in bug 418703 comment 44, but I still have my doubts about this specific case, because it seems self-contradictory: if Windows Mobile has no narrow APIs, when are we going to be using this function? Does WM even have an ANSI code page?
WM does have an ANSI code page.  Its not lacking support for narrow chars.  

on all windows platforms function() gets defined to functionA() of functionW() depending on whether or not UNICODE is defined.

on windows 95/98/ME, fuctionA works and fuctionW returns ERROR_CALL_NOT_IMPLEMENTED

on windows 2000/XP/Vista functionW works and functionA calls functionW after a conversion

on windows mobile functionW works and functionA doesn't link
Comment on attachment 309235 [details] [diff] [review]
directy from "other" patch on bug 418703

r=me, but I filed bug 423828 on maybe removing this altogether
Attachment #309235 - Flags: review?(smontagu) → review+
Assignee

Comment 4

11 years ago
Posted patch fixesSplinter Review
Original patch had a buffer bug, this fixes that and reworks some spacing, and moves one declaration closer to where it is used.  Sorry to ask for mostly-trivial re-review, but there it is.
Assignee: nobody → crowder
Attachment #309235 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #332997 - Flags: review?(smontagu)
Attachment #332997 - Flags: review?(smontagu) → review+
Assignee

Comment 5

11 years ago
changeset:   16564:f5016884eec4
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.