remove redundant charset alias resolution in charset converter manager

RESOLVED FIXED

Status

()

Core
Internationalization
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Jungshik Shin, Assigned: Jungshik Shin)

Tracking

({intl})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

14 years ago
I just found that there's a redundant call to GetCharsetAlias in
GetCharsetLangGroupRaw in nsCharsetConverterManager. |GetCharsetLangGroup| calls
GetCharsetAlias before calling GetCharsetLangGroupRaw so that
GetCharsetLangGroupRaw shouldn't call it again.
(Assignee)

Comment 1

14 years ago
Created attachment 135099 [details] [diff] [review]
patch
(Assignee)

Comment 2

14 years ago
Comment on attachment 135099 [details] [diff] [review]
patch

asking for r/sr.

this redundancy should have been caught when alecf changed the file, but
somehow it was missed.
Attachment #135099 - Flags: superreview?(alecf)
Attachment #135099 - Flags: review?(smontagu)
Comment on attachment 135099 [details] [diff] [review]
patch

r=smontagu
Attachment #135099 - Flags: review?(smontagu) → review+
Comment on attachment 135099 [details] [diff] [review]
patch

sr=bzbarsky
Attachment #135099 - Flags: superreview?(alecf) → superreview+

Comment 5

14 years ago
Comment on attachment 135099 [details] [diff] [review]
patch

oh! nice. sr=alecf

but wait, this doesn't actually remove any redudnant code, except in case of
failure. is there a better way to structure this?
(Assignee)

Comment 6

14 years ago
Thanks for quick r/sr. Fix checked into the trunk.

alecf, without the patch, there were _two_ calls to GetCharsetAlias() in most
cases.  Most consumers call GetCharsetLangGroup which resolves charset alias and
then just passes the resolved charset to GetCharsetLangGroupRaw. Without the
patch, GetCharsetLangGroupRaw calls once again GetCharsetAlias (even though what
it receives is already resolved), which is a waste. Besides, a few consumers who
call GetCharsetLangGroupRaw (because they know what they have is already
resolved) wouldn't benefit from calling Raw version. 
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.