![]() |
|
![]() |
![]() |
|
||
| · Hosting provided by Anduin.net / Øverby Consulting · | ||||||
| Anonymous | Login | Signup for a new account | 2009-11-21 07:08 CET |
| Main | My View | View Issues | Change Log | Roadmap | Docs |
| Viewing Issue Advanced Details [ Jump to Notes ] | [ View Simple ] [ Issue History ] [ Print ] | ||||||
| ID | Category | Severity | Reproducibility | Date Submitted | Last Update | ||
| 0002215 | [Scribus] Internal | major | always | 05-Jul-10 05:00 | 09-Nov-05 21:03 | ||
| Reporter | ringerc | View Status | public | ||||
| Assigned To | jghali | ||||||
| Priority | low | Resolution | fixed | Platform | x86 Linux | ||
| Status | closed | OS | Fedora Core | ||||
| Projection | none | OS Version | 3 | ||||
| ETA | none | Fixed in Version | 1.3.6svn | Product Version | 1.3.3cvs | ||
| Target Version | Product Build | ||||||
| Summary | 0002215: Audit and fix text encoding handling | ||||||
| Description |
There'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. |
||||||
| Steps To Reproduce | |||||||
| Additional Information |
loadText(...) 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. |
||||||
| Tags | No tags attached. | ||||||
| Attached Files | |||||||
|
|
|||||||
Relationships |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Notes |
|
|
(0006278) ringerc (administrator) 05-Aug-29 14:09 |
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. |
|
(0006279) ringerc (administrator) 05-Aug-29 14:15 |
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. |
|
(0006281) ringerc (administrator) 05-Aug-29 14:25 |
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. |
|
(0006282) plinnell (administrator) 05-Aug-29 15:12 |
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. |
|
(0006343) ringerc (administrator) 05-Sep-02 12:53 |
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. |
|
(0006848) plinnell (administrator) 05-Oct-02 17:18 |
bmuping |
|
(0009586) ringerc (administrator) 06-Apr-02 07:52 |
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. |
|
(0009589) ringerc (administrator) 06-Apr-02 08:38 |
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. |
|
(0009594) ringerc (administrator) 06-Apr-02 13:54 edited on: 06-Apr-02 13: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
|
|
(0012059) ringerc (administrator) 06-Jul-31 16:20 |
scribusXml.cpp: QString ScriXmlDoc::WriteElem(....) { ....; return docu.toString().utf8(); } |
|
(0012060) ringerc (administrator) 06-Jul-31 16:24 |
QString StencilReader::createShape(QString datain) { ...; return data.toString().utf8(); } |
|
(0012061) ringerc (administrator) 06-Jul-31 16:25 |
QString StencilReader::createObjects(QString datain) { ...; return data.toString().utf8(); } |
|
(0012062) ringerc (administrator) 06-Jul-31 16:26 |
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; |
|
(0012063) ringerc (administrator) 06-Jul-31 16:28 |
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. |
|
(0012064) ringerc (administrator) 06-Jul-31 16:30 edited on: 06-Jul-31 16: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.) |
|
(0013270) jghali (administrator) 06-Nov-04 14:02 |
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. |
|
(0019431) jghali (administrator) 08-Apr-10 00:18 |
I finally solved that problem in 1.3.5svn by using the gzdopen api + some platform specific code |
|
(0022811) jghali (administrator) 09-Nov-05 21:03 |
Closing as nothing more to do vs that issue was found for quite some time |
| Main | My View | View Issues | Change Log | Roadmap | Docs |
| Mantis 1.1.7[^] Copyright © 2000 - 2008 Mantis Group |