Summary: | [Qt] Height of "Ahem" font differs from all other ports. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||||
Component: | Text | Assignee: | Andreas Kling <kling> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | mitz, ossy, vestbo | ||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Andreas Kling
2011-04-06 08:53:23 PDT
Created attachment 88437 [details]
Proposed patch
*** Bug 57824 has been marked as a duplicate of this bug. *** Comment on attachment 88437 [details]
Proposed patch
If it was me I would include a comment. Seems likely future readers of the code won’t know about the QFontMetricsF quirk.
(In reply to comment #3) > (From update of attachment 88437 [details]) > If it was me I would include a comment. Seems likely future readers of the code won’t know about the QFontMetricsF quirk. Good point, will do that. This patch modifies 3182 tests. :o I'll upload the followup patch here, and we can land them together. Created attachment 89231 [details]
Proposed patch v2
The previous patch exposed a bug on on FreeType platforms where fonts may have negative leading which leads to situations where lineSpacing < (ascent + descent.) Applied the same workaround as SimpleFontDataFreeType to cover this case.
Comment on attachment 89231 [details] Proposed patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=89231&action=review > Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp:61 > + // Workaround from SimpleFontPango.cpp and SimpleFontFreeType.cpp Typo, should be SimpleFontData{Pango,FreeType}.cpp Comment on attachment 89231 [details] Proposed patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=89231&action=review > Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp:66 > + // QFontMetricsF::leading() may return negative values on platforms > + // such as FreeType. Calculate the line gap manually instead. Platform such as freetype? I tested the new patch, the 3168 new results look good to me, except one: diff --git a/LayoutTests/fast/transforms/scrollIntoView-transformed-expected.txt b/LayoutTests/fast/transforms/scrollIntoView-transformed-expected.txt index fde8dfe..78ea7d9 100644 --- a/LayoutTests/fast/transforms/scrollIntoView-transformed-expected.txt +++ b/LayoutTests/fast/transforms/scrollIntoView-transformed-expected.txt @@ -13,5 +13,5 @@ Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor PASS - Element a and Element b had different scrollTop -PASS - Element b had scrollTop: 0 +FAIL - Element b had a non-zero scrollTop: 189 Could you check it? Landed in http://trac.webkit.org/changeset/83871 |