back-compatibility: font name quoting
It looks like recent trunk has begun putting single quotes around font names with spaces in them; I believe this is fine in terms of the CSS spec, except that recent released versions of Inkscape can't parse that and end up using the default font (generally bitstream vera) instead.
Ideas?
-mental
On 6/30/07, MenTaLguY <mental@...3...> wrote:
It looks like recent trunk
...actually it was changed almost 4 months ago... :)
has begun putting single quotes around font names with spaces in them; I believe this is fine in terms of the CSS spec, except that recent released versions of Inkscape can't parse that and end up using the default font (generally bitstream vera) instead.
And why is this a problem? Even without any quotes, those versions failed on a lot of fonts with non-ascii names. See our bug tracker, it's one of the most reported bugs.
On Sat, 2007-06-30 at 10:28 -0400, bulia byak wrote:
...actually it was changed almost 4 months ago... :)
I hadn't been doing serious design work in Inkscape for about four months.
And why is this a problem? Even without any quotes, those versions failed on a lot of fonts with non-ascii names.
Dude.
Most fonts have spaces in their names. That means that nearly all documents saved in 0.46 will be useless text-wise in <= 0.45.x. These are fonts that _worked just fine in 0.45_. In my specific case, I got bitten by "Century Schoolbook L".
Having considered this a bit, I think the best solution is to leave off the single quotes if:
1. the family name has no leading or trailing whitespace 2. the only whitespace chars in the font name are isolated ascii spaces
This is compatible with the CSS parsing rules, and it will allow most fonts that worked in 0.45 to continue working in a backwards-compatible way.
-mental
On 6/30/07, MenTaLguY <mental@...3...> wrote:
Most fonts have spaces in their names. That means that nearly all documents saved in 0.46 will be useless text-wise in <= 0.45.x. These are fonts that _worked just fine in 0.45_.
And they still work just fine _in 0.45_. The only problem is when you move documents from 0.46 to 0.45. And why would you do that? Just upgrade everyone to 0.46 when it's out.
Having considered this a bit, I think the best solution is to leave off the single quotes if:
- the family name has no leading or trailing whitespace
- the only whitespace chars in the font name are isolated ascii spaces
This is compatible with the CSS parsing rules, and it will allow most fonts that worked in 0.45 to continue working in a backwards-compatible way.
I have spent quite some effort on this fix, trying to find the approach which is valid and works with all the weird-named fonts that people attached to their bugreports. (I even had to fix a couple bugs in libcroco to make this work.) Perhaps my fix is not optimal. But please do not try to improve it unless you are willing to dig out all the now-closed bugreports and retest them again after your changes. I honestly don't think it's worth the effort if your only goal is compatibility with the deeply broken 0.45.
On Sat, 2007-06-30 at 17:27 -0400, bulia byak wrote:
The only problem is when you move documents from 0.46 to 0.45. And why would you do that?
Sharing documents with arbitrary people on the web, say?
Just upgrade everyone to 0.46 when it's out.
We can't forcibly upgrade the whole world. It'll eventually settle out, but there's going to be a fairly long period of time when people are deeply unhappy.
There are still a lot of people using _0.44_ to this day.
There's also the issue that this behavior will be perceived as a bug in 0.46, retarding adoption (I've already heard from one user to that effect). It'd be one thing if it was just fonts with non-ASCII characters or something along those lines, but nearly all fonts are affected. It's going to be extremely obvious.
But please do not try to improve it unless you are willing to dig out all the now-closed bugreports and retest them again after your changes.
Can you give me a list? We absolutely do have to fix this.
I honestly don't think it's worth the effort if your only goal is compatibility with the deeply broken 0.45.
...and all previous versions of Inkscape.
-mental
On Sat, 2007-06-30 at 18:46 -0400, MenTaLguY wrote:
We absolutely do have to fix this.
That is, at least for the really common cases. Given how tricky the quoting issues are, I am going to try and make the change very conservative. We can't expect 100% backwards-compatibility with 0.45; I just want to avoid dealing with the fallout from 100% backwards-INcompatibility.
-mental
On Sat, 2007-06-30 at 18:46 -0400, MenTaLguY wrote:
But please do not try to improve it unless you are willing to dig out all the now-closed bugreports and retest them again after your changes.
Can you give me a list? We absolutely do have to fix this.
Along those lines, here's the change I committed. The quoting of font names with international characters isn't altered by this change, so we need only reexamine those bugs which involved fully US-ASCII font names.
Glib::ustring t; bool quote = false; + bool last_was_space = false;
for (gchar const *i = val; *i; i++) { + bool is_space = ( *i == ' ' ); if (g_ascii_isalnum(*i) || *i=='-' || *i=='_') { + // ASCII alphanumeric, - and _ don't require quotes t.push_back(*i); + } else if ( is_space && !last_was_space ) { + // non-consecutive spaces don't require quotes + t.push_back(*i); } else if (*i==''') { + // single quotes require escaping and quotes t.push_back('\'); t.push_back(*i); quote = true; } else { + // everything else requires quotes t.push_back(*i); quote = true; } if (i == val && !g_ascii_isalpha(*i)) { + // a non-ASCII/non-alpha initial character requires quotes quote = true; } + last_was_space = is_space; } + + if (last_was_space) { + // a trailing space requires quotes + quote = true; + }
-mental
On Sat, 2007-06-30 at 19:50 -0400, MenTaLguY wrote:
Along those lines, here's the change I committed. The quoting of font names with international characters isn't altered by this change, so we need only reexamine those bugs which involved fully US-ASCII font names.
So far, I've only been able to find 1688693, 1680744, 1663138, 1655400. Are there any others?
I've verified that there aren't regressions on the first two.
-mental
I've now tested the several weird-named fonts I have on my system gathered from various bug reports, and they seem to work fine. Thanks!
On 6/30/07, MenTaLguY <mental@...3...> wrote:
Glib::ustring t; bool quote = false;
bool last_was_space = false;
for (gchar const *i = val; *i; i++) {
bool is_space = ( *i == ' ' ); if (g_ascii_isalnum(*i) || *i=='-' || *i=='_') {// ASCII alphanumeric, - and _ don't require quotes t.push_back(*i);} else if ( is_space && !last_was_space ) {// non-consecutive spaces don't require quotest.push_back(*i); } else if (*i=='\'') {// single quotes require escaping and quotes t.push_back('\\'); t.push_back(*i); quote = true; } else {// everything else requires quotes t.push_back(*i); quote = true; } if (i == val && !g_ascii_isalpha(*i)) {// a non-ASCII/non-alpha initial character requires quotes quote = true; }last_was_space = is_space;}
if (last_was_space) {
// a trailing space requires quotesquote = true;}
-mental
participants (3)
-
Alexandre Prokoudine -
bulia byak -
MenTaLguY