View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003529 | Scribus | Internal | public | 2006-03-29 12:10 | 2006-04-20 19:28 |
| Reporter | christoph_s | Assigned To | |||
| Priority | high | Severity | major | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | x86 | OS | Linux | OS Version | SuSE 9.3 |
| Product Version | 1.3.3 | ||||
| Fixed in Version | 1.3.4cvs | ||||
| Summary | 0003529: Text frames in scrapbooks lose special characters | ||||
| Description | A text frame stored in a scrapbook loses all special characters like umlauts when dragged to a page. | ||||
| Steps To Reproduce | Create a text frame that contains special chars, add it to a scrapbook. Now drag it from the scrapbook somewhere on a page. Special characters are lost, instead, red squares are shown. | ||||
| Tags | No tags attached. | ||||
| Patch | |||||
|
|
Looks like scrapbook is storing sce files in the system encoding but loads them like they were UTF-8 (converting all the scrapbook sce files to UTF-8 fixes the issue). |
|
|
>>>converting all the scrapbook sce files to UTF-8 fixes the issue I hope this is meant as a hint for developers, not for users. Tell this an average user, and will turn his back on Scribus immediately ;) |
|
|
Your average user doesn't even know this place so they are safe from complicated workarounds. |
|
|
[copied from mail so others' can see the explanation too] I just noticed this fix, and had a look given that I have a bit of experience bashing my head against encoding issues in Scribus. Having had a look over the code, I thought I'd offer my comments. In the following I'm just explaining what's going on. I don't mean to sledge your work and your fix (hell, you did the work, I didn't). However, I do think it's important to understand the situation here and the full effect of what's going on. Your fix actually relies on another bug elsewhere in Scribus - this is a case where Scribus corrupts text when it loads it, then does something else wrong later that balances out. The crux of the problem in this case is that scrap.cpp uses the broken loadText() routine from util.cpp . That's there only for backward compatibility, and really needs to be gotten rid of once all uses of it have been fixed. What it does is read some input byte-by-byte and shove each byte into a QString as if it were a unicode code point. I'm sure you can see how this just won't work for input where the characters associated with those bytes don't match the unicode spec - which in practice means anything except US-ASCII. UTF-8 will get badly corrupted, while I think the latin- codes will usually survive (unless certain characters with special meanings in UTF-16 are present). In most places throughout the Scribus codebase, this mangled QString is demangled by an implicit conversion back to a latin-1 byte string. Those should NEVER EVER EVER happen - they're absolutely always dangerous, and often wrong, since in reality the required string is usually either the output of QString::utf8() or QString::local8Bit() - they're actually a design flaw in Qt. Unfortunately I can't disable these implicit casts without breaking a LOT of existing Scribus code that relies on them. All that code needs to be audited to determine what it actually wants. Consider that QString::fromUtf8 is defined as: static QString QString::fromUtf8 ( const char * utf8, int len = -1 ) ie it doesn't take a QString but rather a const char* . That's partly because converting a QString from utf-8 makes no sense, a QString knows its own encoding and is a true unicode text string. If you pass it a QString anyway, the implicit conversion to const char * provided by QString is enacted: QString::operator const char * () const Which says "Returns ascii().". So what we've done is shoved a bunch of bytes into a QString, then used the implicit conversion to ascii to get them back out. Because Qt's ascii() actually does the same thing as latin1() - passing high-bit-set characters through - this generally works. Just to make things more fun, half the time the saved data is in fact in a corrupt encoding that is corrupted again on loading in such a way as to result in the original data. We often do things like encode utf-8 data as if it was latin-1, then decode it as if it was latin-1 later. Or vice versa. Because of Qt's very permissive text encoding implmementations, this usually works, but it's ugly, and the way it's done (using implicit conversions) often breaks down in different locales. What we should be doing is just putting the bytes into a QByteArray or QCstring in the first place, thus correctly representing the fact that we don't know their encoding, it's "just a string of bytes". QString::fromUtf8(...) then does the right thing presuming that the input is in fact utf-8. The first step is to replace the use of: QString f; loadText(f); with QCString f; loadTextRaw(f); ... then fix the code that uses 'f' to convert it into a QString with the appropriate static method: QString::fromLocal8Bit(...) QString::fromUtf8(...) QString::fromLatin1(...) depending on the encoding of the input text. Sometimes it's necessary to fix the routines that write the data out too, since they're sometimes writing corrupt text data to the file due to an invalid encoding conversion. The scrapbook routines are still doing very bad things with encodings, and they may break in different locales. If you like I'm happy to go through and attack that code. Alternately, if you find my explanation above to be useful, feel free to hack on it yourself - what with my uni and work time pressure I sure won't complain. |
|
|
Re-opening as per my explanation below. Please let me know if you want to tackle it or if I should. I've done it before in other parts of the code, eg file load/save and in prefs, so I'm happy to do it. If you want me to do it just assign to me. |
|
|
We're using QString anyway couldn't we just reimplement the body of the loadText() with the current parameters and convert from QCString to QString safe way. This way we wouldn't need to hack all the places where loadText() is used. fromUTF8() was the evil one in my quick fix. Commenting it out solved the issue. |
|
|
The reason I didn't do loadTextRaw that way originally was that it's just not correct to use a QString for data of unknown encoding. In fact, it's not even possible - to produce a valid QString in all situations you *must* know the encoding of the data that you're loading into it. Without knowing the encoding, it's just a string of bytes with no meaning as text. Now consider a case where the encoding of the input is unknown - perhaps because we need to see a header within the input data in order to determine the encoding. This is the case in many parts of Scribus, including the scrapbook and doc load/save where the name of the root element of the XML document tree (eg <SCRIBUSUTF8NEW>) is examined to determine the document encoding. As it happens that's dodgy - we should be using the <?xml version="1.0" encoding="utf-8"?> directive to set the encoding - but IIRC Qt3 doesn't support that directly, and we need to use the old way for backward compat anyway. In a situation like one of the above, we must read at least part of the file, examine it to determine its encoding, and can only then convert it to a QString. There are several ways to do this: - (a) Move the I/O into the routine that needs the information, so it can read part of the file, determine the encoding, and slurp in the rest. Doesn't really gain us much unless the code is able to benefit from the piece-by-piece data in other ways (eg aborting early on unexpected input instead of reading the whole file first). - (b) Pass an "encoding detector" function to loadText(). Seems like unnecessary complication. - (c) Have loadText() return a byte string, and let the caller do the encoding detection and conversion. I took approach (c) and wrote loadRawText to do just that. It'd be reasonable to have the original loadText() take an encoding argument that tells it what encoding to treat the input data as having, but since in many cases (such as the scrapbook) we can't actually know what encoding it's in until after we've read it, I don't think that's useful. Hope that explanation makes sense. |
|
|
Note that where we _do_ know the encoding, we can usually just use a QTextStream or similar to do the job much more sensibly. |
|
|
confirming, since the issue is there, regardless who and how is going to take care of it:) |
|
|
This actually affects all accented letters in French (so it must affect a lot of other languages, I presume). The feature is hardly usable with text at the moment. Suggestion: raise the priority level from normal to high. |
|
|
Is this still an issue? I thought I fixed this (at least it's working ok with my finnish chars). |
|
|
Confirmed. Works in latest 1.3.3cvs and in 1.3.4cvs with umlauts and eszett. |
|
|
This one was for the scrapbook bug which is fixed now (even if it's a dirty hack). Use 0002215 for general encoding issues. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2006-03-29 12:10 | christoph_s | New Issue | |
| 2006-03-29 21:34 | Tsoots | Note Added: 0009564 | |
| 2006-03-29 22:25 | christoph_s | Note Added: 0009565 | |
| 2006-03-29 22:37 | cbradney | Category | - => Internal |
| 2006-03-29 23:49 | Tsoots | Status | new => assigned |
| 2006-03-29 23:49 | Tsoots | Assigned To | => Tsoots |
| 2006-03-29 23:50 | Tsoots | Status | assigned => resolved |
| 2006-03-29 23:50 | Tsoots | Fixed in Version | => 1.3.4cvs |
| 2006-03-29 23:50 | Tsoots | Resolution | open => fixed |
| 2006-03-29 23:50 | Tsoots | Note Added: 0009566 | |
| 2006-04-02 05:59 |
|
Relationship added | child of 0002215 |
| 2006-04-02 06:32 |
|
Note Added: 0009587 | |
| 2006-04-02 06:33 |
|
Status | resolved => feedback |
| 2006-04-02 06:33 |
|
Resolution | fixed => reopened |
| 2006-04-02 06:33 |
|
Note Added: 0009588 | |
| 2006-04-02 09:01 | Tsoots | Note Added: 0009591 | |
| 2006-04-02 10:15 |
|
Note Added: 0009592 | |
| 2006-04-02 10:21 |
|
Note Edited: 0009592 | |
| 2006-04-02 10:40 |
|
Note Added: 0009593 | |
| 2006-04-20 16:48 | mhanski | Note Added: 0010187 | |
| 2006-04-20 16:48 | mhanski | Assigned To | Tsoots => |
| 2006-04-20 16:48 | mhanski | Status | feedback => confirmed |
| 2006-04-20 18:20 | louisdesjardins | Note Added: 0010190 | |
| 2006-04-20 19:11 | mhanski | Priority | normal => high |
| 2006-04-20 19:11 | mhanski | Severity | minor => major |
| 2006-04-20 19:17 | Tsoots | Note Added: 0010191 | |
| 2006-04-20 19:25 | christoph_s | Note Added: 0010192 | |
| 2006-04-20 19:28 | Tsoots | Status | confirmed => closed |
| 2006-04-20 19:28 | Tsoots | Note Added: 0010194 | |
| 2006-04-20 19:28 | Tsoots | Resolution | reopened => fixed |
