28254 2009-08-13 04:21:27 -0700 [Chromium] Remove legacy code in "WebCore/platform/chromium/PopupMenuChromium.cpp" 2009-08-16 21:26:38 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) PC Linux RESOLVED DUPLICATE 28205 P2 Normal --- 0 hbono webkit-unassigned estade xji oldest_to_newest 139792 0 hbono 2009-08-13 04:21:27 -0700 (Copied from <http://crbug.com/19107>) Chrome Version : 3.0.197.11 OS + version : Ubuntu linux, 8.04 CPU architecture (32-bit / 64-bit): 64 window manager : metacity w/ GNOME URLs (if applicable) : Behavior in Firefox 3.x (if applicable): words typed fast enough refine the selection Behavior in Chrome for Windows (optional): same as Firefox What steps will reproduce the problem? 1. Go to any page which contains a dropdown with more than one entry starting with the same letter 2. Select dropdown and begin typing a word What is the expected result? Given the following options: + |-apple |-anchovy |-antelope |-artichoke |-navel orange |-rutabaga If you type "ar", you should be jumped to "artichoke" What happens instead? You jump to rutabaga. Please use labels and text to provide additional information. Linux and Mac Chromium have not used KeyDown messages since we made our keyboard-input code platform-independent. On the other hand, PopupMenuChromium.cpp still uses the KeyDown messages and this caused the above problem. To solve this problem, we need to remove the platform-dependent code in isCharacterTypeEvent(). 140127 1 hbono 2009-08-13 19:53:40 -0700 I noticed this was a duplicate of <https://bugs.webkit.org/show_bug.cgi?id=28205>. Sorry for committing a duplicate. *** This bug has been marked as a duplicate of bug 28205 *** 140162 2 xji 2009-08-13 22:34:30 -0700 (In reply to comment #1) > I noticed this was a duplicate of > <https://bugs.webkit.org/show_bug.cgi?id=28205>. > Sorry for committing a duplicate. > > *** This bug has been marked as a duplicate of bug 28205 *** This is not a duplicate bug of 28205. fix of 28205 does not remove the platform dependent code in isCharacterTypeEvent(). But I tested the test case in Windows and Mac, they both works fine. Could you reproduce the bug in Linux? 140190 3 hbono 2009-08-14 02:41:47 -0700 (In reply to comment #2) > (In reply to comment #1) > > I noticed this was a duplicate of > > <https://bugs.webkit.org/show_bug.cgi?id=28205>. > > Sorry for committing a duplicate. > > > > *** This bug has been marked as a duplicate of bug 28205 *** > > This is not a duplicate bug of 28205. > fix of 28205 does not remove the platform dependent code in > isCharacterTypeEvent(). I think the solution for this issue is technically the same issue as your Bug 28205. Technically, this issue is caused by this typeAheadFind() that does not update m_lastCharTime at all on Linux Chrome. So, updating m_lastCharTime every time when typeAheadFind() is called is another solution for this issue. I also confirmed this issue does not happen on Linux Chrome now. 140195 4 34825 hbono 2009-08-14 03:42:03 -0700 Created attachment 34825 A layout test that prevent regressions Even though this issue is fixed by a change for Bug 28205, I would like to add a layout test for this bug to prevent regressions. Is it possible to add only a layout test? 140356 5 34825 eric 2009-08-14 15:11:23 -0700 Comment on attachment 34825 A layout test that prevent regressions This bug is resolved. No need for this patch to be marked for review. 140649 6 hbono 2009-08-16 21:26:38 -0700 (In reply to comment #5) > (From update of attachment 34825 [details]) > This bug is resolved. No need for this patch to be marked for review. Sorry for my stupid question in advance. I would like to clarify whether or not it is good to send a change that only includes layout tests since I'm writing some layout tests to replace some part of manual tests with them. As I written in my previous comments, this issue is fixed by a change for Bug 28205. But it uses a manual test because it is not easy to write a layout test which verifies Bug 28205. On the other hand, this issue can be verified with a simple layout test. So, I would like to add this layout test to lessen the dependency to manual tests even though this doesn't include any code changes. Personally, either is fine for me whether or not to add the attached layout test. Nevertheless, I would like to clarify it to prevent annoying reviewers with my stupid changes that only include layout tests. Regards, 34825 2009-08-14 03:42:03 -0700 2009-08-14 15:11:23 -0700 A layout test that prevent regressions issue28254-patch0.txt text/plain 2921 hbono SW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9D aGFuZ2VMb2cJKHJldmlzaW9uIDQ3Mjc1KQorKysgTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMDktMDgtMTQgIEhpcm9ub3JpIEJvbm8gIDxo Ym9ub0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgQWRkIGEgbGF5b3V0IHRlc3QgdGhhdCB2ZXJpZmllcyBCdWcgMjgyNTQuCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yODI1NAorCisg ICAgICAgIEV2ZW4gdGhvdWdoIHRoaXMgY2hhbmdlIGhhcyBiZWVuIGZpeGVkIGJ5IGEgZml4IGZv ciBCdWcgMjgyMDUKKyAgICAgICAgPGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0yODIwNT4sIHRoaXMgY2hhbmdlIGFkZHMKKyAgICAgICAgYSBsYXlvdXQgdGVzdCB0aGF0 IHZlcmlmaWVzIHRoaXMgaXNzdWUgdG8gYXZvaWQgcmVncmVzc2lvbnMuCisKKyAgICAgICAgKiBm YXN0L3NlbGVjdG9ycy90eXBlLWFoZWFkLWZpbmQtZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAgICAg ICAgKiBmYXN0L3NlbGVjdG9ycy90eXBlLWFoZWFkLWZpbmQuaHRtbDogQWRkZWQuCisKIDIwMDkt MDgtMTQgIEFyaXlhIEhpZGF5YXQgIDxhcml5YS5oaWRheWF0QG5va2lhLmNvbT4KIAogICAgICAg ICBSZXZpZXdlZCBieSBUb3IgQXJuZSBWZXN0YsO4LgpJbmRleDogTGF5b3V0VGVzdHMvZmFzdC9z ZWxlY3RvcnMvdHlwZS1haGVhZC1maW5kLWV4cGVjdGVkLnR4dAo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlv dXRUZXN0cy9mYXN0L3NlbGVjdG9ycy90eXBlLWFoZWFkLWZpbmQtZXhwZWN0ZWQudHh0CShyZXZp c2lvbiAwKQorKysgTGF5b3V0VGVzdHMvZmFzdC9zZWxlY3RvcnMvdHlwZS1haGVhZC1maW5kLWV4 cGVjdGVkLnR4dAkocmV2aXNpb24gMCkKQEAgLTAsMCArMSw0IEBACitUaGlzIHRlc3QgdmVyaWZp ZXMgYSBkcm9wLWRvd24gbWVudSBjYW4gcmVmaW5lIHRoZSBzZWxlY3Rpb24gd2hlbiB3ZSBzZW5k IHR3byBvciBtb3JlIGtleWRvd24gZXZlbnRzLgorCisKK1NVQ0NFRURFRApJbmRleDogTGF5b3V0 VGVzdHMvZmFzdC9zZWxlY3RvcnMvdHlwZS1haGVhZC1maW5kLmh0bWwKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g TGF5b3V0VGVzdHMvZmFzdC9zZWxlY3RvcnMvdHlwZS1haGVhZC1maW5kLmh0bWwJKHJldmlzaW9u IDApCisrKyBMYXlvdXRUZXN0cy9mYXN0L3NlbGVjdG9ycy90eXBlLWFoZWFkLWZpbmQuaHRtbAko cmV2aXNpb24gMCkKQEAgLTAsMCArMSw0NiBAQAorPGh0bWw+Cis8aGVhZD4KKyAgICA8dGl0bGU+ PC90aXRsZT4KKzwvaGVhZD4KKzxib2R5PgorICAgIDxwPlRoaXMgdGVzdCB2ZXJpZmllcyBhIGRy b3AtZG93biBtZW51IGNhbiByZWZpbmUgdGhlIHNlbGVjdGlvbiB3aGVuIHdlIHNlbmQgdHdvIG9y IG1vcmUga2V5ZG93biBldmVudHMuPC9wPgorICAgIDxzZWxlY3QgaWQ9InRlc3QiPgorICAgICAg ICA8b3B0aW9uIHZhbHVlPSJhcHBsZSI+YXBwbGU8L29wdGlvbj4KKyAgICAgICAgPG9wdGlvbiB2 YWx1ZT0iYW5jaG92eSI+YW5jaG92eTwvb3B0aW9uPgorICAgICAgICA8b3B0aW9uIHZhbHVlPSJh bnRlbG9wZSI+YW50ZWxvcGU8L29wdGlvbj4KKyAgICAgICAgPG9wdGlvbiB2YWx1ZT0iYXJ0aWNo b2tlIj5hcnRpY2hva2U8L29wdGlvbj4KKyAgICAgICAgPG9wdGlvbiB2YWx1ZT0ibmF2ZWwgb3Jh bmdlIj5uYXZlbCBvcmFuZ2U8L29wdGlvbj4KKyAgICAgICAgPG9wdGlvbiB2YWx1ZT0icnV0YWJh Z2EiPnJ1dGFiYWdhPC9vcHRpb24+CisgICAgPC9zZWxlY3Q+CisgICAgPHVsIGlkPSJjb25zb2xl Ij48L3VsPgorPC9ib2R5PgorPHNjcmlwdD4KK2Z1bmN0aW9uIGxvZyhzdHIpIHsKKyAgICB2YXIg bGkgPSBkb2N1bWVudC5jcmVhdGVFbGVtZW50KCJsaSIpOworICAgIGxpLmFwcGVuZENoaWxkKGRv Y3VtZW50LmNyZWF0ZVRleHROb2RlKHN0cikpOworICAgIHZhciBjb25zb2xlID0gZG9jdW1lbnQu Z2V0RWxlbWVudEJ5SWQoImNvbnNvbGUiKTsKKyAgICBjb25zb2xlLmFwcGVuZENoaWxkKGxpKTsK K30KKworZnVuY3Rpb24gcnVuVGVzdCgpIHsKKyAgICBpZiAod2luZG93LmxheW91dFRlc3RDb250 cm9sbGVyKQorICAgICAgICB3aW5kb3cubGF5b3V0VGVzdENvbnRyb2xsZXIuZHVtcEFzVGV4dCgp OworCisgICAgLy8gU2V0IHRoZSBpbnB1dCBmb2N1cyB0byB0aGUgPHNlbGVjdD4gZWxlbWVudCBh bmQgc2VuZCB0d28ga2V5IGV2ZW50czogJ2EnIGFuZCAncicuCisgICAgdmFyIG5vZGUgPSBkb2N1 bWVudC5nZXRFbGVtZW50QnlJZCgidGVzdCIpOworICAgIG5vZGUuZm9jdXMoKTsKKworICAgIGV2 ZW50U2VuZGVyLmtleURvd24oJ2EnKTsKKyAgICBldmVudFNlbmRlci5rZXlEb3duKCdyJyk7CisK KyAgICAvLyBDb21wYXJlIHRoZSB2YWx1ZSBvZiB0aGlzIDxzZWxlY3Q+IGVsZW1lbnQgd2l0aCB0 aGUgZXhwZWN0ZWQgcmVzdWx0LgorICAgIHZhciBleHBlY3RlZCA9ICdhcnRpY2hva2UnOworICAg IGlmIChub2RlLnZhbHVlID09IGV4cGVjdGVkKQorICAgICAgICBsb2coJ1NVQ0NFRURFRCcpOwor ICAgIGVsc2UKKyAgICAgICAgbG9nKCdGQUlMRUQ6IGV4cGVjdGVkPSInICsgZXhwZWN0ZWQgKyAn LCBhY3R1YWw9IicgKyBub2RlLnZhbHVlICsgJyIuJyk7Cit9CisKK3J1blRlc3QoKTsKKzwvc2Ny aXB0PgorPC9odG1sPgo=