393356 - spellchecker uses two private copies of the Unicode character category table

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix as described

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.

Comment 1

[Boris Zbarsky [:bzbarsky]]

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 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes and yes.

Comment 3

[Simon Montagu :smontagu]

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.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

who's doing intl approvals?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Damon.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked in, and addressed bz's comments.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.