Closed
Bug 393356
Opened 14 years ago
Closed 14 years ago
spellchecker uses two private copies of the Unicode character category table
Categories
(Core :: Spelling checker, defect)
Core
Spelling checker
Tracking
()
RESOLVED
FIXED
People
(Reporter: roc, Assigned: roc)
References
Details
Attachments
(1 file)
25.20 KB,
patch
|
smontagu
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
Spellchecker has its own copy of cattable.h and includes it in two different files, meaning the data is duplicated in our binary. That's bad. I also want to use this table from layout. I'm attaching a patch that moves this table back to intl/ where it belongs and makes spellchecker use it from there.
Attachment #277849 -
Flags: review?(smontagu)
Attachment #277849 -
Flags: approval1.9?
![]() |
||
Comment 1•14 years ago
|
||
Does that mean /extensions/spellcheck/src/cattable.h can go away? And can we stop exporting cattable.h if including it is a bad idea?
Assignee | ||
Comment 2•14 years ago
|
||
Yes and yes.
Comment 3•14 years ago
|
||
Comment on attachment 277849 [details] [diff] [review] fix as described > /** > * Read ftp://ftp.unicode.org/Public/UNIDATA/ReadMe-Latest.txt > * section GENERAL CATEGORY > * for the detail defintation of the following categories > */ Please update this reference to http://www.unicode.org/Public/UNIDATA/UCD.html#General_Category_Values while you're here (and there is also probably a better word than "defintation" ;-) ) >+nsIUGenCategory::nsUGenCategory nsCategoryImp::Get(PRUint32 aChar) > { >- nsUGenCategory cat ; >- PRUint8 ret = GetCat(aChar); >- if( 0 == ret) >- cat = kUGenCategory_Other; // treat it as Cn - Other, Not Assigned >- else >- cat = (nsUGenCategory)ret; >- *oResult = (aCategory == cat ); >- return NS_OK; >+ return nsUGenCategory(GetCat(aChar)); > } FWIW there's a small semantic change here. For example, in WordSplitState::ClassifyCharacter below, undefined codepoints would previously have been treated as separators and now will be treated as word characters.
Attachment #277849 -
Flags: review?(smontagu) → review+
Assignee | ||
Comment 4•14 years ago
|
||
who's doing intl approvals?
![]() |
||
Comment 5•14 years ago
|
||
Damon.
Updated•14 years ago
|
Attachment #277849 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 6•14 years ago
|
||
Checked in, and addressed bz's comments.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 7•14 years ago
|
||
This actually caused a code size increase of 700 bytes --- I think because the intl/ cattable.h is more complete than the one in spellcheck, plus there's a little bit of XPCOM overhead added now. But I think this is OK.
You need to log in
before you can comment on or make changes to this bug.
Description
•