Closed
Bug 1262324
Opened 5 years ago
Closed 5 years ago
Two minor intl/locale/ tweaks
Categories
(Core :: Internationalization, defect)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(2 files)
26.26 KB,
patch
|
emk
:
review+
|
Details | Diff | Splinter Review |
8.33 KB,
patch
|
emk
:
review+
|
Details | Diff | Splinter Review |
I have two minor intl/locale/ tweaks that I stumbled across while looking at the size of Firefox's binary.
![]() |
Assignee | |
Comment 1•5 years ago
|
||
Currently, every entry in unixcharset.properties starts with "locale.all", and then when searching them we prepend "locale.all" to the key. This patch removes the prefixes in both cases, and saves 4 KiB in .text in the process.
Attachment #8738337 -
Flags: review?(VYV03354)
![]() |
Assignee | |
Updated•5 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
![]() |
Assignee | |
Comment 2•5 years ago
|
||
I'm open to other suggestions for the new type name and its field names.
Attachment #8738347 -
Flags: review?(VYV03354)
Comment 3•5 years ago
|
||
Comment on attachment 8738337 [details] [diff] [review] (part 1) - Remove "locale.all" prefix from Unix charsets Nice cleanup.
Attachment #8738337 -
Flags: review?(VYV03354) → review+
Comment 4•5 years ago
|
||
Comment on attachment 8738347 [details] [diff] [review] (part 2) - Introduce nsEncodingProp Review of attachment 8738347 [details] [diff] [review]: ----------------------------------------------------------------- r=me with following comments fixed. ::: dom/encoding/EncodingUtils.cpp @@ +14,5 @@ > > namespace mozilla { > namespace dom { > > +static const nsEncodingProp labelsEncodings[] = { Maybe nsUConvProp is better. Obviously a langGroup is not an encoding. ::: intl/locale/nsUConvPropertySearch.cpp @@ +33,5 @@ > size_t index; > if (BinarySearchIf(aProperties, 0, aNumberOfProperties, > PropertyComparator(flat), &index)) { > + nsDependentCString val(aProperties[index].mValue, > + NS_PTR_TO_UINT32(aProperties[index].mValueLength)); NS_PTR_TO_UINT32 was a trick to store an integer to an element of a pointer array. Now this is unnecessary. ::: intl/locale/nsUConvPropertySearch.h @@ +10,5 @@ > +struct nsEncodingProp > +{ > + const char* const mKey; > + const char* const mValue; > + const int32_t mValueLength; uint32_t. length can't be negative.
Attachment #8738347 -
Flags: review?(VYV03354) → review+
![]() |
Assignee | |
Comment 5•5 years ago
|
||
Thank you for the fast reviews.
![]() |
Assignee | |
Comment 6•5 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/978884d01e8ef968262e54138bf43acdbbc603b6 Bug 1262324 (part 1) - Remove "locale.all" prefix from Unix charsets. r=emk. https://hg.mozilla.org/integration/mozilla-inbound/rev/835b1d9e49182f5a91e9dc931c19b836e14085ba Bug 1262324 (part 2) - Introduce nsUConvProp. r=emk.
Comment 7•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/978884d01e8e https://hg.mozilla.org/mozilla-central/rev/835b1d9e4918
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•