Closed Bug 393356 Opened 12 years ago Closed 12 years ago

spellchecker uses two private copies of the Unicode character category table

Categories

(Core :: Spelling checker, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: roc, Assigned: roc)

References

Details

Attachments

(1 file)

Attached patch fix as describedSplinter 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?
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?
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+
who's doing intl approvals?
Attachment #277849 - Flags: approval1.9? → approval1.9+
Checked in, and addressed bz's comments.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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.