Bug 14086 - Safari 3 beta has severe rendering issues on non-English installs of Windows
: Safari 3 beta has severe rendering issues on non-English installs of Windows
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: New Bugs
: 523.x (Safari 3)
: PC Windows XP
: P1 Critical
Assigned To: Dave Hyatt
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-12 02:26 PDT by Martin Kerz
Modified: 2007-06-14 15:10 PDT (History)
10 users (show)

See Also:


Attachments
Screenshot of badly rendered website in German Windows XP (609.93 KB, image/png)
2007-06-12 02:29 PDT, Martin Kerz
no flags Details
Example of rendering problem on Dutch Windows XP (181.75 KB, image/jpeg)
2007-06-12 02:29 PDT, Niels Leenheer (rakaz)
no flags Details
Screenshot of the same homepage correctly rendered in Safari 3 beta on a Mac (890.25 KB, image/png)
2007-06-12 02:30 PDT, Martin Kerz
no flags Details
Screenshot of the same homepage correctly rendered in Safari 3 beta on an English version of Windows XP (884.78 KB, image/png)
2007-06-12 02:31 PDT, Martin Kerz
no flags Details
Tweakers badly rendered US Vista (267.99 KB, image/png)
2007-06-12 02:55 PDT, Sander Rijken
no flags Details
Use the PostScript name when the full name lookup fails. (4.82 KB, patch)
2007-06-14 02:32 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Improve to try to avoid disappearing text always (5.78 KB, patch)
2007-06-14 13:42 PDT, Dave Hyatt
hyatt: review-
Details | Formatted Diff | Diff
Fix the memory leaks of CFStrings, add code to guard against malformed/malicious font data. (6.38 KB, patch)
2007-06-14 14:21 PDT, Dave Hyatt
mrowe: review-
Details | Formatted Diff | Diff
Fix storage leaks by using a Vector. Do more cleanup and more guards against malicious data. (6.02 KB, patch)
2007-06-14 14:41 PDT, Dave Hyatt
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Kerz 2007-06-12 02:26:17 PDT
Safari 3 beta has severe rendering issues on German and Dutch installs of Windows. 

An example is the the homepage of the German tv station www.zdf.de

On Safari 3 beta for Mac it renders just fine: http://www.kerz.org/files/safari3_mac.png
This is what is produced on Safari 3 beta on Windows XP German (with all latest patches): http://www.kerz.org/files/safari3_win.png

The English version of Windows renders the page correctly: http://www.kerz.org/files/zdf.de-safari3.png

This bug was confirmed on Dutch installations of Windows, too.
Comment 1 Martin Kerz 2007-06-12 02:29:06 PDT
Created attachment 14968 [details]
Screenshot of badly rendered website in German Windows XP
Comment 2 Niels Leenheer (rakaz) 2007-06-12 02:29:25 PDT
Created attachment 14969 [details]
Example of rendering problem on Dutch Windows XP

Added an example of the broken rendering on a Dutch Windows XP installation
Comment 3 Martin Kerz 2007-06-12 02:30:10 PDT
Created attachment 14970 [details]
Screenshot of the same homepage correctly rendered in Safari 3 beta on a Mac
Comment 4 Martin Kerz 2007-06-12 02:31:13 PDT
Created attachment 14971 [details]
Screenshot of the same homepage correctly rendered in Safari 3 beta on an English version of Windows XP
Comment 5 Sander Rijken 2007-06-12 02:54:38 PDT
Problem occurred on a US install as well (on one system not the other).

It occurred on the system that had the regional settings set to 'Dutch' and 'Netherlands'. Changing the unaffected system to those settings didn't change, which might indicate the setting affects the installer in some way.

Attaching a screenshot of badly rendered site on US Windows Vista
Comment 6 Sander Rijken 2007-06-12 02:55:49 PDT
Created attachment 14972 [details]
Tweakers badly rendered US Vista
Comment 7 Torsten Kammer 2007-06-12 12:11:38 PDT
Isn't this the same bug as #14070?
Comment 8 Sander Rijken 2007-06-12 12:19:18 PDT
It might be, but I don't think so, because the same bug occurred on similar installation of vista, only configured differently (Regional Settings) at the time of setup.

Changing that setting afterwards has no effect.

Wouldn't 14070 occur regardless of Regional Settings?
Comment 9 Martin Kerz 2007-06-12 12:36:33 PDT
Seems to me like a duplicate of #14070, yes.
Comment 10 Torsten Kammer 2007-06-12 12:41:13 PDT
I have tested 14070 only on a german installation, so I (In reply to comment #8)
> It might be, but I don't think so, because the same bug occurred on similar
> installation of vista, only configured differently (Regional Settings) at the
> time of setup.
> 
> Changing that setting afterwards has no effect.
> 
> Wouldn't 14070 occur regardless of Regional Settings?
> 

When submitting 14070, I only tested on a german installation of Windows (2000 in this case). I guess I should have written that in there… (I added it now)
Comment 11 Robert Blaut 2007-06-12 14:55:38 PDT
You can fix rendering these rendering issues by changing language for non-Unicode programs. The instruction: http://www.microsoft.com/globaldev/handson/user/xpintlsupp.mspx#EVE

You should restart system after applying setting.
Comment 12 Dave Hyatt 2007-06-14 02:32:51 PDT
Created attachment 15017 [details]
Use the PostScript name when the full name lookup fails.

This patch adds an additional step when the creation of a CGFontRef fails.  If the creation does fail using the full name (which can be localized on non-English installs), we will get the font data of the font and find the PostScript name.

We cache a mapping from localized names to PostScript names so that subsequent lookups don't have to grovel through the font data again.

In my limited testing this fixed my issues.  I tested with Russian and Swedish so far.  I think this should largely fix the "bold/italic" missing text issues.
Comment 13 Oliver Hunt 2007-06-14 02:54:33 PDT
Comment on attachment 15017 [details]
Use the PostScript name when the full name lookup fails.

In addition to irc comments:
+    if (bufferSize != 0) {
should be
+    if (bufferSize) {

While this patch does look "sane" (in as much as that's possible in this case) i'd prefer sfalken/maciej to have a look at it as well.
Comment 14 Dave Hyatt 2007-06-14 03:16:00 PDT
*** Bug 14135 has been marked as a duplicate of this bug. ***
Comment 15 Dave Hyatt 2007-06-14 13:42:13 PDT
Created attachment 15029 [details]
Improve to try to avoid disappearing text always

Even though we'll assert in debug builds if we fail to find a CG font via the PostScript name, this patch adds further bulletproofing to ensure that we fall back to the next font in the list.
Comment 16 Dave Hyatt 2007-06-14 14:21:52 PDT
Created attachment 15031 [details]
Fix the memory leaks of CFStrings, add code to guard against malformed/malicious font data.

Fix leaks and guard against the data in the name table being malicious.
Comment 17 Mark Rowe (bdash) 2007-06-14 14:30:58 PDT
Comment on attachment 15031 [details]
Fix the memory leaks of CFStrings, add code to guard against malformed/malicious font data.

+    if (bufferSize < 6)
+        return NULL;

+    if (bufferSize < stringsOffset)
+        return NULL;

These early returns, and others further on in the function, leak the contents of "buffer".
Comment 18 Dave Hyatt 2007-06-14 14:41:09 PDT
Created attachment 15033 [details]
Fix storage leaks by using a Vector.  Do more cleanup and more guards against malicious data.
Comment 19 Darin Adler 2007-06-14 14:48:04 PDT
Comment on attachment 15033 [details]
Fix storage leaks by using a Vector.  Do more cleanup and more guards against malicious data.

r=me, with the two changes we discussed in person
Comment 20 Dave Hyatt 2007-06-14 15:10:11 PDT
Fixed in r23539.