31103 2009-11-04 00:23:27 -0800 [GTK] editing/selection/shrink-selection-after-shift-pagedown.html failing 2009-12-10 00:29:58 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 xan.lopez webkit-unassigned cshu eric oldest_to_newest 160346 0 xan.lopez 2009-11-04 00:23:27 -0800 This test is failing for at least two reasons. First of all, it tries to modify a textarea without focusing it first, assuming that the DRT will focus the view when starting. Our DRT does not do this, so it fails. Patch attached to fix that. After that is done, the test still fails. Apparently the height we give for the area is too big, so the scrolling code in EditorCommand.cpp goes too far and we end up selecting one row too many (one 'NO' too many), so after doing a shift-up we have still one NO selected. One way to workaround this is to tweak our lineSpacing in our Font code, so I believe this might be related to our age-old bug in that code where our lineSpacing seems to be miscalculated in some cases. I have a small cairo testcase for that (attached) and I'll send it to the cairo list, but in any case this still needs investigation. 160348 1 42458 xan.lopez 2009-11-04 00:28:05 -0800 Created attachment 42458 focusdrt.diff Focus the webviews in DRT when created. Requesting cq also, since I'm not going to be around for a few days. 160349 2 42459 xan.lopez 2009-11-04 00:29:06 -0800 Created attachment 42459 testcairofont.c Small testcase showing how cairo gets linespacing wrong in some situations. 160407 3 42458 jmalonzo 2009-11-04 03:38:52 -0800 Comment on attachment 42458 focusdrt.diff r=me but cq- so Xan can watch for any regressions on the bot, if any. Also, please change the title of this bug and changelog when landing since the patch doesn't fully fix it anyway. 160532 4 cshu 2009-11-04 11:37:54 -0800 (In reply to comment #0) > After that is done, the test still fails. Apparently the height we give for the > area is too big, so the scrolling code in EditorCommand.cpp goes too far and we > end up selecting one row too many (one 'NO' too many), so after doing a > shift-up we have still one NO selected. One way to workaround this is to tweak > our lineSpacing in our Font code, so I believe this might be related to our > age-old bug in that code where our lineSpacing seems to be miscalculated in > some cases. I have a small cairo testcase for that (attached) and I'll send it > to the cairo list, but in any case this still needs investigation. Somehow, I observe the same situation in Qt (see bug 31122). "Shift-PageDown" selected one row more than expected (11 rows all together). However, if I manually do the "Shift-Pagedown" on QtLauncher, the selection is much less: 8 rows. I haven't figured out why. 160620 5 cshu 2009-11-04 14:58:11 -0800 I did some research and found the pagedown scroll amount is platform-dependent. See the following funciton in EditorCommand.cpp: static int verticalScrollDistance(Frame* frame) { ... int height = toRenderBox(renderer)->clientHeight(); return max((height + 1) / 2, height - cAmountToKeepWhenPaging); } I believe height itself depends on the font size. However, cAmountToKeepWhenPaging is hardcoded to 40. Thus, the pagedown event does not guarantee the scroll amount to be the multiplication of line height. Unfortunately, on Qt and GTK, it selected one more line and the test case failed. 160956 6 cshu 2009-11-05 14:42:55 -0800 I fixed the Qt side bug 31122 by changing the test case itself because I think the shift-pagedown behavior on Qt is fine. The patch should fix GTK too. You guys can check once it's landed. 170163 7 eric 2009-12-09 14:05:23 -0800 Ping? Shoudl this bug still be open? 170353 8 xan.lopez 2009-12-10 00:29:58 -0800 Landed the patch, and we seem to be passing the test. Thanks Chang! 42458 2009-11-04 00:28:05 -0800 2009-11-04 03:38:52 -0800 focusdrt.diff focusdrt.diff text/plain 1996 xan.lopez RnJvbSBlMTg0MTNiOTEyOGIwODg3MmM0NzNiNTQ4NDFhMGZhNmI5NGJlN2JiIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBYYW4gTG9wZXogPHhsb3BlekBpZ2FsaWEuY29tPgpEYXRlOiBX ZWQsIDQgTm92IDIwMDkgMTA6MjY6MzcgKzAyMDAKU3ViamVjdDogW1BBVENIXSAyMDA5LTExLTAz ICBYYW4gTG9wZXogIDx4bG9wZXpAaWdhbGlhLmNvbT4KCiAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCgogICAgICAgIFtHVEtdIGVkaXRpbmcvc2VsZWN0aW9uL3Nocmluay1zZWxl Y3Rpb24tYWZ0ZXItc2hpZnQtcGFnZWRvd24uaHRtbCBmYWlsaW5nCiAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMxMTAzCgogICAgICAgIEdpdmUgZm9jdXMg dG8gdGhlIHdlYnZpZXdzIHdoZW4gd2UgY3JlYXRlIHRoZW0sIHNpbmNlIHNvbWUgdGVzdHMgZXhw ZWN0IHRoaXMuCgogICAgICAgICogRHVtcFJlbmRlclRyZWUvZ3RrL0R1bXBSZW5kZXJUcmVlLmNw cDoKICAgICAgICAobWFpbik6Ci0tLQogV2ViS2l0VG9vbHMvQ2hhbmdlTG9nICAgICAgICAgICAg ICAgICAgICAgICAgICAgICB8ICAgMTIgKysrKysrKysrKysrCiBXZWJLaXRUb29scy9EdW1wUmVu ZGVyVHJlZS9ndGsvRHVtcFJlbmRlclRyZWUuY3BwIHwgICAgMSArCiAyIGZpbGVzIGNoYW5nZWQs IDEzIGluc2VydGlvbnMoKyksIDAgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvV2ViS2l0VG9v bHMvQ2hhbmdlTG9nIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCmluZGV4IDE1YjM5ZTkuLmRhM2Rm ZTkgMTAwNjQ0Ci0tLSBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZworKysgYi9XZWJLaXRUb29scy9D aGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAwOS0xMS0wMyAgWGFuIExvcGV6ICA8eGxvcGV6 QGlnYWxpYS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgW0dUS10gZWRpdGluZy9zZWxlY3Rpb24vc2hyaW5rLXNlbGVjdGlvbi1hZnRlci1zaGlm dC1wYWdlZG93bi5odG1sIGZhaWxpbmcKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTMxMTAzCisKKyAgICAgICAgR2l2ZSBmb2N1cyB0byB0aGUgd2Vidmll d3Mgd2hlbiB3ZSBjcmVhdGUgdGhlbSwgc2luY2Ugc29tZSB0ZXN0cyBleHBlY3QgdGhpcy4KKwor ICAgICAgICAqIER1bXBSZW5kZXJUcmVlL2d0ay9EdW1wUmVuZGVyVHJlZS5jcHA6CisgICAgICAg IChtYWluKToKKwogMjAwOS0xMS0wMiAgSm9hbm1hcmllIERpZ2dzICA8am9hbm1hcmllLmRpZ2dz QGdtYWlsLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBYYW4gTG9wZXouCmRpZmYgLS1naXQg YS9XZWJLaXRUb29scy9EdW1wUmVuZGVyVHJlZS9ndGsvRHVtcFJlbmRlclRyZWUuY3BwIGIvV2Vi S2l0VG9vbHMvRHVtcFJlbmRlclRyZWUvZ3RrL0R1bXBSZW5kZXJUcmVlLmNwcAppbmRleCA3MDdi NjFkLi4xMzkyOGQwIDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9EdW1wUmVuZGVyVHJlZS9ndGsv RHVtcFJlbmRlclRyZWUuY3BwCisrKyBiL1dlYktpdFRvb2xzL0R1bXBSZW5kZXJUcmVlL2d0ay9E dW1wUmVuZGVyVHJlZS5jcHAKQEAgLTc5Miw2ICs3OTIsNyBAQCBpbnQgbWFpbihpbnQgYXJnYywg Y2hhciogYXJndltdKQogICAgIGd0a19jb250YWluZXJfYWRkKEdUS19DT05UQUlORVIoY29udGFp bmVyKSwgR1RLX1dJREdFVCh3ZWJWaWV3KSk7CiAgICAgZ3RrX3dpZGdldF9yZWFsaXplKEdUS19X SURHRVQod2ViVmlldykpOwogICAgIGd0a193aWRnZXRfc2hvd19hbGwoY29udGFpbmVyKTsKKyAg ICBndGtfd2lkZ2V0X2dyYWJfZm9jdXMoR1RLX1dJREdFVCh3ZWJWaWV3KSk7CiAgICAgbWFpbkZy YW1lID0gd2Via2l0X3dlYl92aWV3X2dldF9tYWluX2ZyYW1lKHdlYlZpZXcpOwogCiAgICAgc2V0 RGVmYXVsdHNUb0NvbnNpc3RlbnRTdGF0ZVZhbHVlc0ZvclRlc3RpbmcoKTsKLS0gCjEuNi41LjEK Cg== 42459 2009-11-04 00:29:06 -0800 2009-11-04 00:29:06 -0800 testcairofont.c testcairofont.c text/plain 1365 xan.lopez I2luY2x1ZGUgPGNhaXJvLmg+CiNpbmNsdWRlIDxjYWlyby1mdC5oPgojaW5jbHVkZSA8c3RkaW8u aD4KI2luY2x1ZGUgPGFzc2VydC5oPgoKaW50IG1haW4gKGludCBhcmdjLCBjaGFyICphcmd2W10p CnsKICBGY1BhdHRlcm4gKnBhdHRlcm4sICptX3BhdHRlcm47CiAgY29uc3QgY2hhciAqZmNmYW1p bHk7CiAgY29uc3QgY2Fpcm9fZm9udF9vcHRpb25zX3QqIG9wdGlvbnM7CiAgRmNSZXN1bHQgZmNy ZXN1bHQ7CiAgY2Fpcm9fbWF0cml4X3QgY3RtOwogIGNhaXJvX2ZvbnRfZmFjZV90KiBmb250RmFj ZTsKICBjYWlyb19tYXRyaXhfdCBmb250TWF0cml4OwogIGNhaXJvX3NjYWxlZF9mb250X3QgKm1f c2NhbGVkRm9udDsKICBjYWlyb19mb250X2V4dGVudHNfdCBmb250X2V4dGVudHM7CgogIHBhdHRl cm4gPSBGY1BhdHRlcm5DcmVhdGUgKCk7CiAgb3B0aW9ucyA9IGNhaXJvX2ZvbnRfb3B0aW9uc19j cmVhdGUgKCk7CiAgZmNmYW1pbHkgPSAibW9ub3NwYWNlIjsKCiAgRmNQYXR0ZXJuQWRkU3RyaW5n KHBhdHRlcm4sIEZDX0ZBTUlMWSwgKEZjQ2hhcjgqKWZjZmFtaWx5KTsKICBGY0NvbmZpZ1N1YnN0 aXR1dGUoTlVMTCwgcGF0dGVybiwgRmNNYXRjaFBhdHRlcm4pOwogIEZjRGVmYXVsdFN1YnN0aXR1 dGUocGF0dGVybik7CgogIG1fcGF0dGVybiA9IEZjRm9udE1hdGNoKE5VTEwsIHBhdHRlcm4sICZm Y3Jlc3VsdCk7CiAgZm9udEZhY2UgPSBjYWlyb19mdF9mb250X2ZhY2VfY3JlYXRlX2Zvcl9wYXR0 ZXJuKG1fcGF0dGVybik7CgovLyBQYXNzaW5nIGEgc2NhbGUgb2YgMTMgc2hvd3MgdGhlIGJ1Zwoj ZGVmaW5lIFNDQUxFIDEzLjAKICBjYWlyb19tYXRyaXhfaW5pdF9zY2FsZSgmZm9udE1hdHJpeCwg U0NBTEUsIFNDQUxFKTsKICBjYWlyb19tYXRyaXhfaW5pdF9pZGVudGl0eSgmY3RtKTsKCiAgb3B0 aW9ucyA9IGNhaXJvX2ZvbnRfb3B0aW9uc19jcmVhdGUoKTsKICBtX3NjYWxlZEZvbnQgPSBjYWly b19zY2FsZWRfZm9udF9jcmVhdGUoZm9udEZhY2UsICZmb250TWF0cml4LCAmY3RtLCBvcHRpb25z KTsKICBjYWlyb19mb250X2ZhY2VfZGVzdHJveShmb250RmFjZSk7CiAgRmNQYXR0ZXJuRGVzdHJv eShwYXR0ZXJuKTsKCiAgY2Fpcm9fc2NhbGVkX2ZvbnRfZXh0ZW50cyhtX3NjYWxlZEZvbnQsICZm b250X2V4dGVudHMpOwogIHByaW50ZiAoIkFTQ0VOVCAlZiwgREVTQ0VOVCAlZiwgTElORSBIRUlH SFQgJWZcbiIsIGZvbnRfZXh0ZW50cy5hc2NlbnQsIGZvbnRfZXh0ZW50cy5kZXNjZW50LCBmb250 X2V4dGVudHMuaGVpZ2h0KTsKICBhc3NlcnQgKGZvbnRfZXh0ZW50cy5oZWlnaHQgPj0gZm9udF9l eHRlbnRzLmFzY2VudCArIGZvbnRfZXh0ZW50cy5kZXNjZW50KTsKCiAgcmV0dXJuIDA7Cn0K