Bug 13711 - REGRESSION - Bookmark Bar text rendering changed
Summary: REGRESSION - Bookmark Bar text rendering changed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: mitz
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2007-05-14 08:09 PDT by Gibbons Burke
Modified: 2007-05-16 12:08 PDT (History)
2 users (show)

See Also:


Attachments
Screen capture comparing bookmark bars in Safari.app and Webkit.app (38.28 KB, image/gif)
2007-05-14 08:12 PDT, Gibbons Burke
no flags Details
Replaces Attachment 14548 - Updated screen capture - PNG - better resolution (45.17 KB, image/png)
2007-05-14 08:19 PDT, Gibbons Burke
no flags Details
Avoid synthetic bold and italic in the chrome (3.54 KB, patch)
2007-05-14 13:47 PDT, mitz
darin: review+
Details | Formatted Diff | Diff
Screen capture showing the side by side QuadCam images (227.88 KB, image/png)
2007-05-16 12:07 PDT, Gibbons Burke
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gibbons Burke 2007-05-14 08:09:41 PDT
The attached screen shot illustrates the difference between the rendering of the text on the bookmarks bar in the nightly build vs. the current release of Safari. In the nightly build, the text looks smudged, especially in the symbol characters I use to represent various URLs such as the circled cross character or the parallel wavy lines. The crisp detail of those characters is lost in the new version.
Comment 1 Gibbons Burke 2007-05-14 08:12:06 PDT
Created attachment 14548 [details]
Screen capture comparing bookmark bars in Safari.app and Webkit.app

In this image the controls for Safari are on top and Webkit.app on the bottom. Note how 'smudged' and out of focus the text appears in WebKit.app, especially on the special characters which have fine detail.
Comment 2 Gibbons Burke 2007-05-14 08:19:58 PDT
Created attachment 14549 [details]
Replaces Attachment 14548 [details] - Updated screen capture - PNG - better resolution

This higher resolution PNG image shows better the difference between how Safari.app and WebKit.app render the characters in the Bookmarks Bar. The WebKit regression 'smudges' the text and obscures details in the character forms.
Comment 3 mitz 2007-05-14 09:14:27 PDT
Looks like synthetic bold, which shouldn't be done for chrome.
Comment 4 mitz 2007-05-14 13:47:58 PDT
Created attachment 14552 [details]
Avoid synthetic bold and italic in the chrome
Comment 5 Darin Adler 2007-05-14 16:15:57 PDT
Comment on attachment 14552 [details]
Avoid synthetic bold and italic in the chrome

r=me
Comment 6 Mark Rowe (bdash) 2007-05-14 19:01:58 PDT
Landed in r21471.
Comment 7 Darin Adler 2007-05-16 09:41:27 PDT
I think I was a little hasty reviewing this. It's true that the chrome should not have synthetic bold, but I don't think the platform font code path really needs a flag to tell it not to synthesize bold. Instead, I think we should not have synthesized bold because the font is already bold. Or something along those lines. I would like to do a better fix at some point.

Or I could be convinced this fix was correct.

In any case, it's good to have the correct behavior, even if it might be for the wrong reason. So I don't regret that we landed the change.
Comment 8 mitz 2007-05-16 10:06:09 PDT
(In reply to comment #7)
> I think we should not
> have synthesized bold because the font is already bold.

I'm not sure which font "the font" refers to. The platform font was Lucida Grande Bold, which has the bold trait. The fallback font for, e.g., the AQUARIUS character (U+2652) is Apple Symbol Regular, which does not have the bold trait. That's when the font cache decides to use synthetic bold, and that's when the patch says that it shouldn't.

> Or I could be convinced this fix was correct.

Did I convince you?

I think it may have been clearer if the Font had a flag named "do not synthesize missing traits in fallback fonts", but in practice this coincides with "this Font was constructed from a FontPlatformData".
Comment 9 Darin Adler 2007-05-16 11:01:53 PDT
(In reply to comment #8)
> I think it may have been clearer if the Font had a flag named "do not
> synthesize missing traits in fallback fonts", but in practice this coincides
> with "this Font was constructed from a FontPlatformData".

Yes, I think it's sort of a "coincidence" that we want this behavior in Safari UI, and also happen to use the FontPlatformData code path in that case, so a separate boolean would be clearer. The reason we don't want to synthesize bold has something to do with how Lucida Grande Bold looks and what fallback fonts we typically get.

Thanks for explaining. Maybe we should just add a comment to the fallback code -- I don't think any code changes are needed, but the fallback code logic is not self explanatory.
Comment 10 Gibbons Burke 2007-05-16 12:07:27 PDT
Created attachment 14582 [details]
Screen capture showing the side by side QuadCam images

I'm unable to persuade Snapz Pro to save the movie image I capture to disk so this is a still capture. I tried to capture one of the glitch frames but was unable to do so. But one thing to notice is that the WebKit.app window on the left's URL shows the page is still loading. In Safari on the right this isn't an issue.
Comment 11 Gibbons Burke 2007-05-16 12:08:48 PDT
Comment on attachment 14582 [details]
Screen capture showing the side by side QuadCam images

Apologies -- should be associated with bug 
13642