View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0002215ScribusInternalpublic2005-07-10 03:002009-11-05 20:03
ReporterringercAssigned Tojghali  
PrioritylowSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Platformx86 LinuxOSFedora CoreOS Version3
Product Version1.3.3cvs 
Fixed in Version1.3.6svn 
Summary0002215: Audit and fix text encoding handling
DescriptionThere's lots of very odd text handling in Scribus. I've already found and fixed some that was broken, but I think there's a lot more there.

All uses of QString::utf(), QString::ascii(), QString::latin1(), QString::fromUtf8(), etc need to be checked. More generally, anywhere where we go from a byte string to a QString or vice versa needs examining.

These issues can not be fixed in isolation. Often, something is incorrect but works (or "mostly works") because it's doing something wrong twice in a reversable way. QString is also very forgiving with bad encoding conversions, alas.
Additional InformationloadText(...) in util.cpp is wrong. It shoves data byte-by-byte into a QString. Functions later on then demangle the QString by using implicit conversion to QCString. I'm pretty sure there are bad double-conversions going on here, they just end up working. loadText(...) should use a QCString. All users of loadText will need work.

Prefs loading in 1.2.3 needs fixing; the current prefs file is saved mangled.

SXW / ODT import and friends (anything that uses the zip code) needs checking for filename handling.

scrap.cpp BibView::drabObject(...) does a bad unicode conversion:
- QString dt = objectMap[currentItem()->text()].Data.utf8();
+ QString dt(objectMap[currentItem()->text()].Data);
but more checking is needed to see if it works because of a bad conversion somewhere else, or if it's just broken.

Lots more to come, this is just from early checking.
TagsNo tags attached.
Patch

Relationships

related to 0002452 closedringerc question marks instead of cyrillic letters in startup splash screens 
related to 0002728 closedringerc Fix bad encoding handling in FileLoader, ScribusXML's .sla loading 
related to 0002950 closedringerc scribus --help > Unicode glyphs replaced by ? 
parent of 0000349 closedringerc If a file is saved under a default name under a non-English locale it does not show up in the "Recent documents" submenu. 
parent of 0002120 closedringerc Some sort of jpeg images can't be loaded 
parent of 0002518 closedringerc Prefs corruption in 1.2.x 
parent of 0003529 closed Text frames in scrapbooks lose special characters 
related to 0002376 closedringerc Source code requests 
related to 0002382 acknowledged Metabug: Code quality, structure, readability 
related to 0002397 closedfschmid Regression: EPS with clipping path does not work anymore with 1.3.0 
related to 0002774 closedringerc Silently stops at launch during ICC profiles readings (International characters) 

Activities

ringerc

2005-08-29 12:09

reporter   ~0006278

Also need to check for cases of tr() being used on a QString. It must always be used on a `const char*' or QCString in latin-1 encoding.

Where more than latin-1 is required, a utf-8 `const char*' or QCString should be translated by using the utf-8 version of `tr' on a unicode byte string generated by QString::utf8() or by entering the byte string as '\x00' escapes.

Anywhere that QString is implicitly converted to `const char*' or QCString is a potential bug.

ringerc

2005-08-29 12:15

reporter   ~0006279

http://doc.trolltech.com/qq/qq05-achtung.html is useful.

If we can make Scribus compile cleanly with QT_NO_ASCII_CAST then many of these issues will go away. It'll be a bunch of work though, and won't fix cases of incorrect explicit conversions like what was causing the prefs corruption.

ringerc

2005-08-29 12:25

reporter   ~0006281

Until it's added as a config option, you can use:

./configure --enable-debug CXXFLAGS=-DQT_NO_ASCII_CAST=1

to build with that flag set. Note that the build WILL fail until a lot of cleanup work is done.

plinnell

2005-08-29 13:12

viewer   ~0006282

Note: if you have 1.2.3cvs and 1.3.1cvs installed and no others installed, 1.3.1cvs keeps asking to convert prefs on opening.

ringerc

2005-09-02 10:53

reporter   ~0006343

Looks like file loading / saving only works because of matched pairs of mistakes, too... there's lots of:

QString something = SomeOtherQString.utf8();

and

QString something = QString::fromUtf8(SomeOtherQString);

... which almost always indicates a bug.

plinnell

2005-10-02 15:18

viewer   ~0006848

bmuping

ringerc

2006-04-02 05:52

reporter   ~0009586

Note that the worst of the user-visible encoding issues are now resolved. There's still a lot of dirty QString abuse behind the scenes, though - PDFLib, for example, relies entirely on corrupt QStrings, and there are still users of the old broken file reader that loads data into a mangled QString. This bug has ceased to be a priority, however, and future cases will get fixed along with other improvements throughout the code.

ringerc

2006-04-02 06:38

reporter   ~0009589

Bad loadText() use is still everywhere. Please ignore the >--- tab indicators in the following search results from the early 1.3.4cvs codebase:

grep -n loadText $(find -name \*.cpp ) | grep -v loadTextRaw
./serializer.cpp:180:>--bool tmp = loadText(Filename, &Objekt);
./scribusXml.cpp:1828:>->---if (!loadText(file, &f))
./scribusXml.cpp:1876:>->---if (!loadText(fileName, &f))
./customfdialog.cpp:193:>--->---if (loadText(name, &Buffer))
./pdflib.cpp:983:>-->---loadText(ScMW->InputProfiles[Options.SolidProf], &dataP);
./pdflib.cpp:4522:>->--->---loadText(fn, &im);
./pdflib.cpp:4707:>->--->--->--->--->--->---loadText((Embedded ? ScMW->InputProfilesCMYK[Options.ImageProf] : ScMW->InputProfilesCMYK[Profil]), &dataP);
./pdflib.cpp:4712:>->--->--->--->--->--->---loadText((Embedded ? ScMW->InputProfiles[Options.ImageProf] : ScMW->InputProfiles[Profil]), &dataP);
./pdflib.cpp:4721:>->--->--->--->--->---loadText((Embedded ? ScMW->InputProfilesCMYK[Options.ImageProf] : ScMW->InputProfilesCMYK[Profil]), &dataP);
./pdflib.cpp:4726:>->--->--->--->--->---loadText((Embedded ? ScMW->InputProfiles[Options.ImageProf] : ScMW->InputProfiles[Profil]), &dataP);
./pdflib.cpp:4863:>->--->--->---loadText(fn, &im);
./pdflib.cpp:4869:>->--->--->---loadText(fn, &im);
./pdflib.cpp:4891:>->--->--->--->---loadText(tmpFile, &im);
./pdflib.cpp:4922:>->--->--->--->--->---loadText(tmpFile, &im);
./pdflib.cpp:4931:>->--->--->--->---loadText(tmpFile, &im);
./pdflib.cpp:5335:>->---loadText(PrintPr, &dataP);
./scrap.cpp:68:>>---loadText(dt, &f);
./scrap.cpp:76:>>---loadText(dt, &f);
./scrap.cpp:101:>--->--->---if (!loadText(QDir::cleanDirPath(QDir::convertSeparators(oldName + "/" + d[dc])), &f))
./scrap.cpp:130:>--->--->---if (!loadText(QDir::cleanDirPath(QDir::convertSeparators(oldName + "/" + d2[dc])), &f))
./scrap.cpp:159:>---if (!loadText(name, &f))
./scrap.cpp:207:>--->--->---if (!loadText(QDir::cleanDirPath(QDir::convertSeparators(name + "/" + d[dc])), &f))
./scrap.cpp:232:>--->--->---if (!loadText(QDir::cleanDirPath(QDir::convertSeparators(name + "/" + d2[dc])), &f))
./scrap.cpp:587:>--->--->---if (loadText(ur.path(), &text))
./story.cpp:1083:void SEditor::loadText(QString tx, PageItem *currItem)
./story.cpp:3613:>-->--->--->---Editor->loadText(data, currItem);
./plugins/fileloader/oodraw/oodrawimp.cpp:212:>->---loadText(stylePath, &f);
./plugins/fileloader/oodraw/oodrawimp.cpp:215:>->---loadText(contentPath, &f2);
./plugins/fileloader/oodraw/oodrawimp.cpp:225:>->--->---loadText(metaPath, &f3);
./plugins/gettext/textfilter/textfilter.cpp:52:>>---loadText();
./plugins/gettext/textfilter/textfilter.cpp:60:void TextFilter::loadText()
./plugins/gettext/txtim/txtim.cpp:41:>--loadText();
./plugins/gettext/txtim/txtim.cpp:50:void TxtIm::loadText()
./plugins/scriptplugin/objprinter.cpp:26:bool SCRIBUS_API loadText(QString nam, QString *Buffer);
./plugins/scriptplugin/objprinter.cpp:150:>-// loadText is defined in utils.cpp
./plugins/scriptplugin/objprinter.cpp:151:>-if (loadText("/etc/printcap", &Pcap))
./plugins/svgimplugin/svgplugin.cpp:206:>--->---loadText(fName, &f);
./plugins/svgimplugin/svgplugin.cpp:208:>---loadText(fName, &f);
./pslib.cpp:683:>--->---if (loadText(fn, &tmp))
./pslib.cpp:750:>--->---if (loadText(fn, &tmp))
./util.cpp:311:bool loadText(QString filename, QString *Buffer)

Each and every loadText use is at best ugly, and is quite likely to indicate a bug, possibly a locale-dependent bug.

ringerc

2006-04-02 11:54

reporter   ~0009594

Last edited: 2006-04-02 11:55

 Potentially major bugs at the end of scribusXml.cpp (around line 3356):

#ifdef HAVE_LIBZ
    if(fileName.right(2) == "gz")     {
        // zipped saving
        // XXX: latin1() should probably be local8Bit()
        gzFile gzDoc = gzopen(fileName.latin1(),"wb");
        if(gzDoc == NULL)
            return false;
        gzputs(gzDoc, docu.toString().utf8());
        gzclose(gzDoc);
    }
    else
    {
        // duplicate code to below
        QFile f(fileName);
        if(!f.open(IO_WriteOnly))
            return false;
        QTextStream s(&f);
        // implicit conversion of `const char*' or `QCString' containing
        // utf8 to QString, probably as latin-1
        QString wr = docu.toString().utf8();
        // writeRawBytes() with implicit conversion of QString to latin-1
        // but using length() of QString not byte-string representation.
        s.writeRawBytes(wr, wr.length());
        f.close();
    }
#else
    QFile f(fileName);
    if(!f.open(IO_WriteOnly))
        return false;
    QTextStream s(&f);
    QString wr = docu.toString().utf8();     s.writeRawBytes(wr, wr.length());
    f.close();
#endif


ringerc

2006-07-31 14:20

reporter   ~0012059

scribusXml.cpp:

 QString ScriXmlDoc::WriteElem(....) { ....; return docu.toString().utf8(); }

ringerc

2006-07-31 14:24

reporter   ~0012060

QString StencilReader::createShape(QString datain) {
   ...; return data.toString().utf8();
}

ringerc

2006-07-31 14:25

reporter   ~0012061

QString StencilReader::createObjects(QString datain) {
 ...;
 return data.toString().utf8();
}

ringerc

2006-07-31 14:26

reporter   ~0012062

void BibView::ReadOldContents(QString name, QString newName)
{
    QDomDocument docu("scridoc");
    QString f = "";
    if (!loadText(name, &f))
        return;
    QString ff = "";
    // these were corrupting the scrapbook entries, removed and works ok now, Riku
// if (f.startsWith("<SCRIBUSSCRAPUTF8"))
// ff = QString::fromUtf8(f);
// else
        ff = f;
    if(!docu.setContent(ff))
        return;

ringerc

2006-07-31 14:28

reporter   ~0012063

Reminder sent to: jghali

Jean,

You seem to have some understanding of encoding issues, so I've taken the liberty of putting you on the monitor list for this bug. I use this to keep track of encoding issues I find (and when I get a chance, fix) so there's some interesting and dodgy stuff here.

ringerc

2006-07-31 14:30

reporter   ~0012064

Last edited: 2006-07-31 14:33

 void Biblio::ObjFromMenu(QString text)
{
    QString nam, tmp;
    ...
    tmp = text;
    QString ff = "";
    // these were corrupting the encoding, by removing it should be now ok, we'll see, Riku
// if (tmp.startsWith("<SCRIBUSELEMUTF8"))
// ff = QString::fromUtf8(tmp);
// else
       ff = tmp;

(This one is actually fine - Riku has commented out the broken code; I initially misread it as doing an implicit QCString to QString conversion but it's not. It's still unnecessarily convoluted.)

jghali

2006-11-04 13:02

administrator   ~0013270

I made yesterday further tests on Windows of some parts of code using latin1() function. As expected the following code fails to open a file as soon as the filename path contains unicode characters :

if(fileName.right(2) == "gz") {
    // XXX: latin1() should probably be local8Bit()
    gzFile gzDoc;
    gzDoc = gzopen(fileName.latin1(),"rb");
    if(gzDoc == NULL)
    {
        // FIXME: Needs better error return
        return false;
    }
    ...
}

But more interestingly, i came to a point where replacing the latin1() call did not show any improvement: local8Bit(), utf8(), ascii() all failed... Even tried ucs2() ;) As soon as gzopen call was avoided by saving non compressed file, the file could be loaded successfully from the very same directory, the file being opened in this case using Windows unicode i/o functions.

jghali

2008-04-09 22:18

administrator   ~0019431

I finally solved that problem in 1.3.5svn by using the gzdopen api + some platform specific code

jghali

2009-11-05 20:03

administrator   ~0022811

Closing as nothing more to do vs that issue was found for quite some time

Issue History

Date Modified Username Field Change
2005-07-10 03:00 ringerc New Issue
2005-07-10 03:01 ringerc Relationship added parent of 0000349
2005-07-10 03:01 ringerc Relationship added parent of 0002120
2005-07-10 03:03 ringerc Additional Information Updated
2005-08-03 15:50 ringerc Relationship added related to 0002376
2005-08-03 16:27 ringerc Relationship added related to 0002382
2005-08-09 15:20 ringerc Relationship added related to 0002397
2005-08-29 12:06 ringerc Relationship added related to 0002452
2005-08-29 12:09 ringerc Note Added: 0006278
2005-08-29 12:15 ringerc Note Added: 0006279
2005-08-29 12:25 ringerc Note Added: 0006281
2005-08-29 13:12 plinnell Note Added: 0006282
2005-09-02 09:58 ringerc Relationship added parent of 0002518
2005-09-02 10:53 ringerc Note Added: 0006343
2005-10-02 15:18 plinnell Note Added: 0006848
2005-10-02 15:18 plinnell Product Version 1.3.1cvs => 1.3.2cvs
2005-10-16 16:35 ringerc Relationship added related to 0002728
2005-10-30 10:08 ringerc Relationship added related to 0002774
2006-01-02 14:04 ringerc Relationship added related to 0002950
2006-01-16 22:51 plinnell Product Version 1.3.2cvs => 1.3.3cvs
2006-04-02 05:52 ringerc Note Added: 0009586
2006-04-02 05:54 ringerc Priority high => low
2006-04-02 05:59 ringerc Relationship added parent of 0003529
2006-04-02 06:38 ringerc Note Added: 0009589
2006-04-02 11:54 ringerc Note Added: 0009594
2006-04-02 11:55 ringerc Note Edited: 0009594
2006-07-31 14:20 ringerc Note Added: 0012059
2006-07-31 14:24 ringerc Note Added: 0012060
2006-07-31 14:25 ringerc Note Added: 0012061
2006-07-31 14:26 ringerc Note Added: 0012062
2006-07-31 14:28 ringerc Note Added: 0012063
2006-07-31 14:30 ringerc Note Added: 0012064
2006-07-31 14:33 ringerc Note Edited: 0012064
2006-11-04 13:02 jghali Note Added: 0013270
2007-01-22 02:42 ringerc Relationship added related to 0005216
2008-03-14 06:46 ringerc Assigned To ringerc =>
2008-03-14 06:52 ringerc Status assigned => acknowledged
2008-04-09 22:18 jghali Note Added: 0019431
2008-04-09 22:28 jghali Relationship deleted related to 0005216
2009-11-05 20:03 jghali Note Added: 0022811
2009-11-05 20:03 jghali Status acknowledged => resolved
2009-11-05 20:03 jghali Fixed in Version => 1.3.6svn
2009-11-05 20:03 jghali Resolution open => fixed
2009-11-05 20:03 jghali Assigned To => jghali
2009-11-05 20:03 jghali Status resolved => closed