Re: Patch: Fix for Bug 1164


Subject: Re: Patch: Fix for Bug 1164
From: Hubert Figuiere (hfiguiere@teaser.fr)
Date: Mon May 21 2001 - 04:59:44 CDT


According to Hubert Figuiere <hfiguiere@teaser.fr>:
> According to Andrew Dunbar <hippietrail@yahoo.com>:
> > This patch enables the RTF importer to make us of the \fcharset and
> > \fcpg properties of the font table and switch between encodings
> > when it encounters \f.
> >
> > There are a few edge cases and charsets I couldn't find information
> > on so please contact me or implement these if you know about them.
>
>
> OK I take it. I will do some testing before committing it, but it looks good.

In fact your patch is not OK as it breaks handling of charset defined
by \mac, \ansi, \pc and \pca keywords.
(see lines 1572, 1578, 1716, 1765 and 1770)
See bug 836
        <http://bugzilla.abisource.com/show_bug.cgi?id=836>

Not all test case I have break, but most of of them fail to decode properly
the Mac charset, and on ASSERTs:

DEBUG: RTF Font has neither codepage *nor* charset
DEBUG: RTF Font has neither codepage *nor* charset
DEBUG: RTF Font has neither codepage *nor* charset

**** (4) Assert ****
**** (4) pOld == NULL at ie_imp_RTF.cpp:2927 ****
**** (4) Continue ? (y/n) [y] :
DEBUG: RTF Font has neither codepage *nor* charset
DEBUG: RTF Font has neither codepage *nor* charset

**** (5) Assert ****
**** (5) pOld == NULL at ie_imp_RTF.cpp:2927 ****
**** (5) Continue ? (y/n) [y] :
DEBUG: RTF Font has neither codepage *nor* charset

**** (6) Assert ****
**** (6) pOld == NULL at ie_imp_RTF.cpp:2927 ****
**** (6) Continue ? (y/n) [y] :
DEBUG: RTF Font has neither codepage *nor* charset

Basically from what I can see is that you assume that the font should
have a codepage or a charset. This is false as only the charset
specification (see above) should be enough for most cases, and this
is actually what is done by lot of old RTF writers.
Only text that does not have font attribute is properly decoded.

I can send you sample files if you really need.

The bottom line: I will reject the patch unless someone fixes it. I'm
sorry. I'd really like to see 1164 fixed too.

Hub



This archive was generated by hypermail 2b25 : Sat May 26 2001 - 03:51:05 CDT