45942 2010-09-16 20:05:05 -0700 [GTK] fast/forms/listbox-selection.html fails 2010-09-20 11:16:08 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC Linux RESOLVED FIXED Gtk P3 Normal --- 1 mrobinson webkit-unassigned abarth dbates eric webkit.review.bot xan.lopez oldest_to_newest 280515 0 mrobinson 2010-09-16 20:05:05 -0700 It appears the issue is that keyDown in the EventSender does not handle all the appropriate modifier strings. 280516 1 67877 mrobinson 2010-09-16 20:07:55 -0700 Created attachment 67877 Patch for this issue 280888 2 67877 dbates 2010-09-17 14:57:40 -0700 Comment on attachment 67877 Patch for this issue View in context: https://bugs.webkit.org/attachment.cgi?id=67877&action=prettypatch This patch looks sane to me. r=me > LayoutTests/platform/gtk/Skipped:-1169 > -fast/forms/listbox-selection.html I did a brief search for all layout tests that call eventSender.keyDown() with a non-empty list of modifiers that are also in the GTK Skipped list and found fast/events/keydown-1.html. Does this still fail? > WebKitTools/DumpRenderTree/gtk/EventSender.cpp:463 > + // Get the modifiers from the second argument. > + guint modifiers = argumentCount >= 2 ? gdkModifersFromJSValue(context, arguments[1]) : 0; I noticed that you did not put a similar comment "// Get the modifiers from the second argument." above the other call sites in mouseUpCallback and mouseDownCallback. We may want to consider putting such a comment at these other call sites. 281650 3 mrobinson 2010-09-20 10:43:05 -0700 (In reply to comment #2) > I did a brief search for all layout tests that call eventSender.keyDown() with a non-empty list of modifiers that are also in the GTK Skipped list and found fast/events/keydown-1.html. Does this still fail? This one just needed platform-specific results. I've added them. > I noticed that you did not put a similar comment "// Get the modifiers from the second argument." above the other call sites in mouseUpCallback and mouseDownCallback. We may want to consider putting such a comment at these other call sites. I just removed this comment. After further consideration it really answers "What?" instead of "Why?" It's superfluous, I think. 281651 4 mrobinson 2010-09-20 10:43:25 -0700 (In reply to comment #3) > (In reply to comment #2) > > I did a brief search for all layout tests that call eventSender.keyDown() with a non-empty list of modifiers that are also in the GTK Skipped list and found fast/events/keydown-1.html. Does this still fail? > > This one just needed platform-specific results. I've added them. > > > I noticed that you did not put a similar comment "// Get the modifiers from the second argument." above the other call sites in mouseUpCallback and mouseDownCallback. We may want to consider putting such a comment at these other call sites. > > I just removed this comment. After further consideration it really answers "What?" instead of "Why?" It's superfluous, I think. By the way, thanks for the awesome review! 281652 5 mrobinson 2010-09-20 10:44:20 -0700 Committed r67860: <http://trac.webkit.org/changeset/67860> 281681 6 webkit.review.bot 2010-09-20 11:16:08 -0700 http://trac.webkit.org/changeset/67860 might have broken GTK Linux 32-bit Release 67877 2010-09-16 20:07:55 -0700 2010-09-17 14:57:40 -0700 Patch for this issue bug-45942-20100916200754.patch text/plain 4557 mrobinson ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA0OTllMTM3NzM4N2I0YzhlZWU5MWNmZjM1ODAxYmRjZTUxZjczYjI1Li41NTk0ODAw MzdjOWVmMWViMGI1MTQ1Yzc1NzRmNDE0YWZmOGQ4YmRjIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0 cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTIgQEAK KzIwMTAtMDktMTYgIE1hcnRpbiBSb2JpbnNvbiAgPG1yb2JpbnNvbkBpZ2FsaWEuY29tPgorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFtHVEtdIGZhc3Qv Zm9ybXMvbGlzdGJveC1zZWxlY3Rpb24uaHRtbCBmYWlscworICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDU5NDIKKworICAgICAgICAqIHBsYXRmb3JtL2d0 ay9Ta2lwcGVkOiBVbnNraXAgdGVzdCB3aGljaCBpcyBub3cgcGFzc2luZy4KKwogMjAxMC0wOS0x NiAgVG9ueSBDaGFuZyAgPHRvbnlAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5 IE9qYW4gVmFmYWkuCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9wbGF0Zm9ybS9ndGsvU2tpcHBl ZCBiL0xheW91dFRlc3RzL3BsYXRmb3JtL2d0ay9Ta2lwcGVkCmluZGV4IGEwOGU0MWRjOTMzMDU1 NGM1MjRmMjNmYzM1ZWI1MjhiZGIzNjY5MjQuLjg4Mzk3Yjg0YWJmNjUyNDg0MDMxNzNhOGUyYzZi MjdlMjY3OWE2NmQgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL3BsYXRmb3JtL2d0ay9Ta2lwcGVk CisrKyBiL0xheW91dFRlc3RzL3BsYXRmb3JtL2d0ay9Ta2lwcGVkCkBAIC0xMTY2LDcgKzExNjYs NiBAQCBmYXN0L2Zvcm1zL2dldC1maWxlLXVwbG9hZC5odG1sCiBmYXN0L2Zvcm1zL2lucHV0LWZp bGUtcmUtcmVuZGVyLmh0bWwKIGZhc3QvZm9ybXMvbGVnZW5kLWFjY2Vzcy1rZXkuaHRtbAogZmFz dC9mb3Jtcy9saXN0Ym94LW9uY2hhbmdlLmh0bWwKLWZhc3QvZm9ybXMvbGlzdGJveC1zZWxlY3Rp b24uaHRtbAogZmFzdC9mb3Jtcy9vbmNoYW5nZS1lbnRlci1zdWJtaXQuaHRtbAogZmFzdC9mb3Jt cy9vbnNlbGVjdC10ZXh0YXJlYS5odG1sCiBmYXN0L2Zvcm1zL29uc2VsZWN0LXRleHRmaWVsZC5o dG1sCmRpZmYgLS1naXQgYS9XZWJLaXRUb29scy9DaGFuZ2VMb2cgYi9XZWJLaXRUb29scy9DaGFu Z2VMb2cKaW5kZXggMDM5ZGMwMTJlOGMwMWI2NzBiOTM5ZjFmZmNiN2U5YTI2ZmY2MGE3Zi4uMzVm NjkwMTZkY2E4MDdkM2Y4NDgwY2JiMDE4MjgzNDcxYTQwYzgwNiAxMDA2NDQKLS0tIGEvV2ViS2l0 VG9vbHMvQ2hhbmdlTG9nCisrKyBiL1dlYktpdFRvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3 IEBACisyMDEwLTA5LTE2ICBNYXJ0aW4gUm9iaW5zb24gIDxtcm9iaW5zb25AaWdhbGlhLmNvbT4K KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbR1RLXSBm YXN0L2Zvcm1zL2xpc3Rib3gtc2VsZWN0aW9uLmh0bWwgZmFpbHMKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQ1OTQyCisKKyAgICAgICAgVXNlIHRoZSBn ZGtNb2RpZmVyc0Zyb21KU1ZhbHVlIGhlbHBlciB0byBwYXJzZSBhbGwgYXBwcm9wcmlhdGUgbW9k aWZpZXIKKyAgICAgICAgc3RyaW5ncyBpbiBrZXlEb3duQ2FsbGJhY2suCisKKyAgICAgICAgKiBE dW1wUmVuZGVyVHJlZS9ndGsvRXZlbnRTZW5kZXIuY3BwOgorICAgICAgICAoa2V5RG93bkNhbGxi YWNrKTogVXNlIHRoZSBnZGtNb2RpZmVyc0Zyb21KU1ZhbHVlIGluc3RlYWQgb2YgZHVwbGljYXRp bmcgdGhlIAorICAgICAgICBtb2RpZmllciBwYXJzaW5nIGxvZ2ljLgorCiAyMDEwLTA5LTE1ICBU b255IENoYW5nICA8dG9ueUBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgS2Vu dCBUYW11cmEuCmRpZmYgLS1naXQgYS9XZWJLaXRUb29scy9EdW1wUmVuZGVyVHJlZS9ndGsvRXZl bnRTZW5kZXIuY3BwIGIvV2ViS2l0VG9vbHMvRHVtcFJlbmRlclRyZWUvZ3RrL0V2ZW50U2VuZGVy LmNwcAppbmRleCAwZTJlNDY5YWFkMjI0MjI0NzE2NTA3Njc3MzQxZDczOGZhNjA4YjZjLi4yNzkz NmJmMDhiZjk0ZTU5ODkyZjk1ZTNiZmFlYTJlNTM0OWZjMDg1IDEwMDY0NAotLS0gYS9XZWJLaXRU b29scy9EdW1wUmVuZGVyVHJlZS9ndGsvRXZlbnRTZW5kZXIuY3BwCisrKyBiL1dlYktpdFRvb2xz L0R1bXBSZW5kZXJUcmVlL2d0ay9FdmVudFNlbmRlci5jcHAKQEAgLTQ1OSwyOSArNDU5LDggQEAg c3RhdGljIEpTVmFsdWVSZWYga2V5RG93bkNhbGxiYWNrKEpTQ29udGV4dFJlZiBjb250ZXh0LCBK U09iamVjdFJlZiBmdW5jdGlvbiwgSlMKICAgICBpZiAoYXJndW1lbnRDb3VudCA8IDEpCiAgICAg ICAgIHJldHVybiBKU1ZhbHVlTWFrZVVuZGVmaW5lZChjb250ZXh0KTsKIAotICAgIHN0YXRpYyBj b25zdCBKU1N0cmluZ1JlZiBsZW5ndGhQcm9wZXJ0eSA9IEpTU3RyaW5nQ3JlYXRlV2l0aFVURjhD U3RyaW5nKCJsZW5ndGgiKTsKLQotICAgIHdlYmtpdF93ZWJfZnJhbWVfbGF5b3V0KG1haW5GcmFt ZSk7Ci0KLSAgICAvLyBoYW5kbGUgbW9kaWZpZXIga2V5cy4KLSAgICBpbnQgc3RhdGUgPSAwOwot ICAgIGlmIChhcmd1bWVudENvdW50ID4gMSkgewotICAgICAgICBKU09iamVjdFJlZiBtb2RpZmll cnNBcnJheSA9IEpTVmFsdWVUb09iamVjdChjb250ZXh0LCBhcmd1bWVudHNbMV0sIGV4Y2VwdGlv bik7Ci0gICAgICAgIGlmIChtb2RpZmllcnNBcnJheSkgewotICAgICAgICAgICAgZm9yIChpbnQg aSA9IDA7IGkgPCBKU1ZhbHVlVG9OdW1iZXIoY29udGV4dCwgSlNPYmplY3RHZXRQcm9wZXJ0eShj b250ZXh0LCBtb2RpZmllcnNBcnJheSwgbGVuZ3RoUHJvcGVydHksIDApLCAwKTsgKytpKSB7Ci0g ICAgICAgICAgICAgICAgSlNWYWx1ZVJlZiB2YWx1ZSA9IEpTT2JqZWN0R2V0UHJvcGVydHlBdElu ZGV4KGNvbnRleHQsIG1vZGlmaWVyc0FycmF5LCBpLCAwKTsKLSAgICAgICAgICAgICAgICBKU1N0 cmluZ1JlZiBzdHJpbmcgPSBKU1ZhbHVlVG9TdHJpbmdDb3B5KGNvbnRleHQsIHZhbHVlLCAwKTsK LSAgICAgICAgICAgICAgICBpZiAoSlNTdHJpbmdJc0VxdWFsVG9VVEY4Q1N0cmluZyhzdHJpbmcs ICJjdHJsS2V5IikpCi0gICAgICAgICAgICAgICAgICAgIHN0YXRlIHw9IEdES19DT05UUk9MX01B U0s7Ci0gICAgICAgICAgICAgICAgZWxzZSBpZiAoSlNTdHJpbmdJc0VxdWFsVG9VVEY4Q1N0cmlu ZyhzdHJpbmcsICJzaGlmdEtleSIpKQotICAgICAgICAgICAgICAgICAgICBzdGF0ZSB8PSBHREtf U0hJRlRfTUFTSzsKLSAgICAgICAgICAgICAgICBlbHNlIGlmIChKU1N0cmluZ0lzRXF1YWxUb1VU RjhDU3RyaW5nKHN0cmluZywgImFsdEtleSIpKQotICAgICAgICAgICAgICAgICAgICBzdGF0ZSB8 PSBHREtfTU9EMV9NQVNLOwotCi0gICAgICAgICAgICAgICAgSlNTdHJpbmdSZWxlYXNlKHN0cmlu Zyk7Ci0gICAgICAgICAgICB9Ci0gICAgICAgIH0KLSAgICB9CisgICAgLy8gR2V0IHRoZSBtb2Rp ZmllcnMgZnJvbSB0aGUgc2Vjb25kIGFyZ3VtZW50LgorICAgIGd1aW50IG1vZGlmaWVycyA9IGFy Z3VtZW50Q291bnQgPj0gMiA/IGdka01vZGlmZXJzRnJvbUpTVmFsdWUoY29udGV4dCwgYXJndW1l bnRzWzFdKSA6IDA7CiAKICAgICAvLyBoYW5kbGUgbG9jYXRpb24gYXJndW1lbnQuCiAgICAgaW50 IGxvY2F0aW9uID0gRE9NX0tFWV9MT0NBVElPTl9TVEFOREFSRDsKQEAgLTU3NCw3ICs1NTMsNyBA QCBzdGF0aWMgSlNWYWx1ZVJlZiBrZXlEb3duQ2FsbGJhY2soSlNDb250ZXh0UmVmIGNvbnRleHQs IEpTT2JqZWN0UmVmIGZ1bmN0aW9uLCBKUwogICAgICAgICAgICAgZWxzZSB7CiAgICAgICAgICAg ICAgICAgZ2RrS2V5U3ltID0gZ2RrX3VuaWNvZGVfdG9fa2V5dmFsKGNoYXJDb2RlKTsKICAgICAg ICAgICAgICAgICBpZiAoV1RGOjppc0FTQ0lJVXBwZXIoY2hhckNvZGUpKQotICAgICAgICAgICAg ICAgICAgICBzdGF0ZSB8PSBHREtfU0hJRlRfTUFTSzsKKyAgICAgICAgICAgICAgICAgICAgbW9k aWZpZXJzIHw9IEdES19TSElGVF9NQVNLOwogICAgICAgICAgICAgfQogICAgICAgICB9CiAgICAg fQpAQCAtNTg3LDcgKzU2Niw3IEBAIHN0YXRpYyBKU1ZhbHVlUmVmIGtleURvd25DYWxsYmFjayhK U0NvbnRleHRSZWYgY29udGV4dCwgSlNPYmplY3RSZWYgZnVuY3Rpb24sIEpTCiAgICAgLy8gY3Jl YXRlIGFuZCBzZW5kIHRoZSBldmVudAogICAgIEdka0V2ZW50KiBwcmVzc0V2ZW50ID0gZ2RrX2V2 ZW50X25ldyhHREtfS0VZX1BSRVNTKTsKICAgICBwcmVzc0V2ZW50LT5rZXkua2V5dmFsID0gZ2Rr S2V5U3ltOwotICAgIHByZXNzRXZlbnQtPmtleS5zdGF0ZSA9IHN0YXRlOworICAgIHByZXNzRXZl bnQtPmtleS5zdGF0ZSA9IG1vZGlmaWVyczsKICAgICBwcmVzc0V2ZW50LT5rZXkud2luZG93ID0g Z3RrX3dpZGdldF9nZXRfd2luZG93KEdUS19XSURHRVQodmlldykpOwogICAgIGdfb2JqZWN0X3Jl ZihwcmVzc0V2ZW50LT5rZXkud2luZG93KTsKICNpZm5kZWYgR1RLX0FQSV9WRVJTSU9OXzIK