Closed Bug 393356 Opened 14 years ago Closed 14 years ago

spellchecker uses two private copies of the Unicode character category table


(Core :: Spelling checker, defect)

Not set





(Reporter: roc, Assigned: roc)




(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
>     *  section GENERAL CATEGORY
>     *  for the detail defintation of the following categories
>     */

Please update this reference to 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.
Closed: 14 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.