57954 2011-04-06 08:53:23 -0700 [Qt] Height of "Ahem" font differs from all other ports. 2011-04-14 10:59:20 -0700 1 1 1 Unclassified WebKit Text 528+ (Nightly build) All All RESOLVED FIXED Qt, QtTriaged P2 Normal --- 1 kling kling mitz ossy vestbo oldest_to_newest 380420 0 kling 2011-04-06 08:53:23 -0700 This is caused by the behavior of QFontMetricsF::descent(), which returns the descent minus one, for historical reasons. 380436 1 88437 kling 2011-04-06 09:00:18 -0700 Created attachment 88437 Proposed patch 380458 2 ossy 2011-04-06 09:10:01 -0700 *** Bug 57824 has been marked as a duplicate of this bug. *** 380476 3 88437 darin 2011-04-06 09:26:32 -0700 Comment on attachment 88437 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. 380492 4 kling 2011-04-06 09:39:16 -0700 (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. 380508 5 ossy 2011-04-06 09:48:58 -0700 This patch modifies 3182 tests. :o I'll upload the followup patch here, and we can land them together. 384186 6 89231 kling 2011-04-12 11:31:19 -0700 Created attachment 89231 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. 384195 7 89231 kling 2011-04-12 11:37:52 -0700 Comment on attachment 89231 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 385002 8 89231 benjamin 2011-04-13 10:57:00 -0700 Comment on attachment 89231 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? 385691 9 ossy 2011-04-14 05:58:50 -0700 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? 385878 10 ossy 2011-04-14 10:59:07 -0700 Landed in http://trac.webkit.org/changeset/83871 88437 2011-04-06 09:00:18 -0700 2011-04-12 11:31:19 -0700 Proposed patch bug-57954.diff text/plain 1477 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA1MzI4MjU4Li4yY2I1Y2IyIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkg QEAKKzIwMTEtMDQtMDYgIEFuZHJlYXMgS2xpbmcgIDxrbGluZ0B3ZWJraXQub3JnPgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFtRdF0gSGVpZ2h0IG9m ICJBaGVtIiBmb250IGRpZmZlcnMgZnJvbSBhbGwgb3RoZXIgcG9ydHMuCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01Nzk1NAorCisgICAgICAgIFFGb250 TWV0cmljc0Y6OmRlc2NlbnQoKSByZXR1cm5zIHRoZSBhY3R1YWwgZGVzY2VudCBtaW51cyBvbmUs CisgICAgICAgIGZvciBoaXN0b3JpY2FsIHJlYXNvbnMuICBXZSBuZWVkIHRvIGFkZCBpdCBiYWNr IHRvIGdldCBjb3JyZWN0CisgICAgICAgIG1ldHJpY3MgZm9yIFdlYktpdC4KKworICAgICAgICBU aGlzIGlzIGNvdmVyZWQgYnkgbW9zdCBsYXlvdXQgdGVzdHMuCisKKyAgICAgICAgKiBwbGF0Zm9y bS9ncmFwaGljcy9xdC9TaW1wbGVGb250RGF0YVF0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNp bXBsZUZvbnREYXRhOjpwbGF0Zm9ybUluaXQpOgorCiAyMDExLTA0LTAxICBBbGV4YW5kZXIgUGF2 bG92ICA8YXBhdmxvdkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgWXVyeSBT ZW1pa2hhdHNreS4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNz L3F0L1NpbXBsZUZvbnREYXRhUXQuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvcXQvU2ltcGxlRm9udERhdGFRdC5jcHAKaW5kZXggNWQwYjMwMi4uNGIyNmNhOCAxMDA2NDQK LS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvU2ltcGxlRm9udERhdGFR dC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvU2ltcGxlRm9u dERhdGFRdC5jcHAKQEAgLTQ5LDcgKzQ5LDcgQEAgdm9pZCBTaW1wbGVGb250RGF0YTo6cGxhdGZv cm1Jbml0KCkKIAogICAgIFFGb250TWV0cmljc0YgZm0obV9wbGF0Zm9ybURhdGEuZm9udCgpKTsK ICAgICBtX2ZvbnRNZXRyaWNzLnNldEFzY2VudChmbS5hc2NlbnQoKSk7Ci0gICAgbV9mb250TWV0 cmljcy5zZXREZXNjZW50KGZtLmRlc2NlbnQoKSk7CisgICAgbV9mb250TWV0cmljcy5zZXREZXNj ZW50KGZtLmRlc2NlbnQoKSArIDEpOwogICAgIG1fZm9udE1ldHJpY3Muc2V0WEhlaWdodChmbS54 SGVpZ2h0KCkpOwogICAgIG1fZm9udE1ldHJpY3Muc2V0TGluZUdhcChmbS5sZWFkaW5nKCkpOwog ICAgIG1fZm9udE1ldHJpY3Muc2V0TGluZVNwYWNpbmcoZm0ubGluZVNwYWNpbmcoKSk7Cg== 89231 2011-04-12 11:31:19 -0700 2011-04-14 10:59:20 -0700 Proposed patch v2 bug-57954-v2.diff text/plain 2998 kling ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA1ZWZmMWZlLi5jNWUwNWI2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjQg QEAKKzIwMTEtMDQtMTIgIEFuZHJlYXMgS2xpbmcgIDxrbGluZ0B3ZWJraXQub3JnPgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFtRdF0gSGVpZ2h0IG9m ICJBaGVtIiBmb250IGRpZmZlcnMgZnJvbSBhbGwgb3RoZXIgcG9ydHMuCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01Nzk1NAorCisgICAgICAgIFFGb250 TWV0cmljc0Y6OmRlc2NlbnQoKSByZXR1cm5zIHRoZSBhY3R1YWwgZGVzY2VudCBtaW51cyAxLAor ICAgICAgICB0byBhY2NvdW50IGZvciB0aGUgYmFzZWxpbmUuIEFkZCBpdCBiYWNrIHRvIGdldCBj b3JyZWN0IG1ldHJpY3MKKyAgICAgICAgZm9yIFdlYktpdC4KKworICAgICAgICBRRm9udE1ldHJp Y3NGOjpsZWFkaW5nKCkgbWF5IHJldHVybiBuZWdhdGl2ZSB2YWx1ZXMgb24gc29tZQorICAgICAg ICBwbGF0Zm9ybXMgKHRob3NlIHVzaW5nIEZyZWVUeXBlKSwgdGhpcyBicmVha3MgV2ViS2l0J3Mg YXNzdW1wdGlvbgorICAgICAgICB0aGF0IGxpbmVTcGFjaW5nID49IChhc2NlbnQgKyBkZXNjZW50 KSwgc28gd2Ugd29yayBhcm91bmQgdGhpcyBpbgorICAgICAgICB0aGUgc2FtZSBmYXNoaW9uIGFz IFNpbXBsZUZvbnREYXRhRnJlZVR5cGUgYW5kIFNpbXBsZUZvbnREYXRhUGFuZ28uCisKKyAgICAg ICAgVGhpcyBpcyBjb3ZlcmVkIGJ5IHRob3VzYW5kcyBvZiBsYXlvdXQgdGVzdHMuCisKKyAgICAg ICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9xdC9TaW1wbGVGb250RGF0YVF0LmNwcDoKKyAgICAgICAg KFdlYkNvcmU6OlNpbXBsZUZvbnREYXRhOjpwbGF0Zm9ybUluaXQpOgorCiAyMDExLTA0LTEyICBQ YXZlbCBGZWxkbWFuICA8cGZlbGRtYW5AZ29vZ2xlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBi eSBZdXJ5IFNlbWlraGF0c2t5LgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v Z3JhcGhpY3MvcXQvU2ltcGxlRm9udERhdGFRdC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9ncmFwaGljcy9xdC9TaW1wbGVGb250RGF0YVF0LmNwcAppbmRleCA1ZDBiMzAyLi45YmYxMDMw IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9TaW1wbGVG b250RGF0YVF0LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9T aW1wbGVGb250RGF0YVF0LmNwcApAQCAtMSw1ICsxLDUgQEAKIC8qCi0gICAgQ29weXJpZ2h0IChD KSAyMDA4IE5va2lhIENvcnBvcmF0aW9uIGFuZC9vciBpdHMgc3Vic2lkaWFyeSgtaWVzKQorICAg IENvcHlyaWdodCAoQykgMjAwOCwgMjAwOSwgMjAxMCwgMjAxMSBOb2tpYSBDb3Jwb3JhdGlvbiBh bmQvb3IgaXRzIHN1YnNpZGlhcnkoLWllcykKICAgICBDb3B5cmlnaHQgKEMpIDIwMDggSG9sZ2Vy IEhhbnMgUGV0ZXIgRnJleXRoZXIKIAogICAgIFRoaXMgbGlicmFyeSBpcyBmcmVlIHNvZnR3YXJl OyB5b3UgY2FuIHJlZGlzdHJpYnV0ZSBpdCBhbmQvb3IKQEAgLTQ4LDExICs0OCwyOSBAQCB2b2lk IFNpbXBsZUZvbnREYXRhOjpwbGF0Zm9ybUluaXQoKQogICAgIH0KIAogICAgIFFGb250TWV0cmlj c0YgZm0obV9wbGF0Zm9ybURhdGEuZm9udCgpKTsKLSAgICBtX2ZvbnRNZXRyaWNzLnNldEFzY2Vu dChmbS5hc2NlbnQoKSk7Ci0gICAgbV9mb250TWV0cmljcy5zZXREZXNjZW50KGZtLmRlc2NlbnQo KSk7CisKKyAgICAvLyBRdCBzdWJ0cmFjdHMgMSBmcm9tIHRoZSBkZXNjZW50IHRvIGFjY291bnQg Zm9yIHRoZSBiYXNlbGluZSwKKyAgICAvLyB3ZSBhZGQgaXQgYmFjayBoZXJlIHRvIGdldCBjb3Jy ZWN0IG1ldHJpY3MgZm9yIFdlYktpdC4KKyAgICBmbG9hdCBkZXNjZW50ID0gZm0uZGVzY2VudCgp ICsgMTsKKyAgICBmbG9hdCBhc2NlbnQgPSBmbS5hc2NlbnQoKTsKKworICAgIGZsb2F0IGxpbmVT cGFjaW5nID0gZm0ubGluZVNwYWNpbmcoKTsKKworICAgIC8vIFRoZSBsaW5lIHNwYWNpbmcgc2hv dWxkIGFsd2F5cyBiZSA+PSAoYXNjZW50ICsgZGVzY2VudCksIGJ1dCB0aGlzCisgICAgLy8gbWF5 IGJlIGZhbHNlIGluIHNvbWUgY2FzZXMgZHVlIHRvIG1pc2JlaGF2aW5nIHBsYXRmb3JtIGxpYnJh cmllcy4KKyAgICAvLyBXb3JrYXJvdW5kIGZyb20gU2ltcGxlRm9udFBhbmdvLmNwcCBhbmQgU2lt cGxlRm9udEZyZWVUeXBlLmNwcAorICAgIGlmIChsaW5lU3BhY2luZyA8IGFzY2VudCArIGRlc2Nl bnQpCisgICAgICAgIGxpbmVTcGFjaW5nID0gYXNjZW50ICsgZGVzY2VudDsKKworICAgIC8vIFFG b250TWV0cmljc0Y6OmxlYWRpbmcoKSBtYXkgcmV0dXJuIG5lZ2F0aXZlIHZhbHVlcyBvbiBwbGF0 Zm9ybXMKKyAgICAvLyBzdWNoIGFzIEZyZWVUeXBlLiBDYWxjdWxhdGUgdGhlIGxpbmUgZ2FwIG1h bnVhbGx5IGluc3RlYWQuCisgICAgZmxvYXQgbGluZUdhcCA9IGxpbmVTcGFjaW5nIC0gYXNjZW50 IC0gZGVzY2VudDsKKworICAgIG1fZm9udE1ldHJpY3Muc2V0QXNjZW50KGFzY2VudCk7CisgICAg bV9mb250TWV0cmljcy5zZXREZXNjZW50KGRlc2NlbnQpOworICAgIG1fZm9udE1ldHJpY3Muc2V0 TGluZVNwYWNpbmcobGluZVNwYWNpbmcpOwogICAgIG1fZm9udE1ldHJpY3Muc2V0WEhlaWdodChm bS54SGVpZ2h0KCkpOwotICAgIG1fZm9udE1ldHJpY3Muc2V0TGluZUdhcChmbS5sZWFkaW5nKCkp OwotICAgIG1fZm9udE1ldHJpY3Muc2V0TGluZVNwYWNpbmcoZm0ubGluZVNwYWNpbmcoKSk7Cisg ICAgbV9mb250TWV0cmljcy5zZXRMaW5lR2FwKGxpbmVHYXApOwogICAgIG1fc3BhY2VXaWR0aCA9 IGZtLndpZHRoKFFMYXRpbjFDaGFyKCcgJykpOwogfQogCg==