36923 2010-03-31 19:40:50 -0700 [Chromium]: FindInPage issue with focus remaining on element after match is found 2010-04-01 09:45:56 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 0 finnur.webkit webkit-unassigned commit-queue progame+wk webkit.review.bot oldest_to_newest 206906 0 finnur.webkit 2010-03-31 19:40:50 -0700 In Chromium: 1. Go to a page that has a text input field at the top, such as http://groups.google.com/?pli=1 2. Ctrl-F 3. Type a string that will match somewhere lower on the page (so that the page scrolls down when a match is found) 4. Click on any link on the page Notice that the page scrolls back up and sometimes the navigation doesn't occur. This is because in WebViewImpl::SetFocus we update the focus appearance for textfields, to work around a focus bug. This call causes the textfield at the top of the page to scroll into view, which is not what we want. We don't actually need the textfield to have focus once we find a match so the fix is simply to clear the focus during find (if a match is found). I will submit a patch shortly. 206908 1 52243 finnur.webkit 2010-03-31 19:46:00 -0700 Created attachment 52243 Patch for Find issue 36923 206914 2 webkit.review.bot 2010-03-31 19:58:50 -0700 Attachment 52243 did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/chromium/src/WebFrameImpl.cpp:1327: Use 0 instead of NULL. [readability/null] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 206915 3 52245 finnur.webkit 2010-03-31 20:01:48 -0700 Created attachment 52245 Now with style issue fixed 207166 4 52245 ojan 2010-04-01 08:16:31 -0700 Comment on attachment 52245 Now with style issue fixed Code looks correct to me. > Index: WebKit/chromium/ChangeLog > =================================================================== > --- WebKit/chromium/ChangeLog (revision 56886) > +++ WebKit/chromium/ChangeLog (working copy) > @@ -1,3 +1,15 @@ > +2010-03-31 Finnur Thorarinsson <finnur.webkit@gmail.com> > + > + Reviewed by NOBODY (OOPS!). > + > + [chromium] FindInPage should clear the focused node when a match has > + been found. > + > + https://bugs.webkit.org/show_bug.cgi?id=36923 Would be nice to have a description here about how this fixes the bug, i.e. that WebFrameImpl::setFocus will try to refocus editable elements if it thinks they have focus, causing the page to scroll. 207194 5 52303 finnur.webkit 2010-04-01 09:13:56 -0700 Created attachment 52303 Updated description 207201 6 52303 dglazkov 2010-04-01 09:26:59 -0700 Comment on attachment 52303 Updated description ok. 207207 7 52303 commit-queue 2010-04-01 09:45:51 -0700 Comment on attachment 52303 Updated description Clearing flags on attachment: 52303 Committed r56917: <http://trac.webkit.org/changeset/56917> 207208 8 commit-queue 2010-04-01 09:45:56 -0700 All reviewed patches have been landed. Closing bug. 52243 2010-03-31 19:46:00 -0700 2010-03-31 20:01:48 -0700 Patch for Find issue 36923 find_focus_fix.patch text/plain 1331 finnur.webkit SW5kZXg6IFdlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViS2l0L2No cm9taXVtL0NoYW5nZUxvZwkocmV2aXNpb24gNTY4ODYpCisrKyBXZWJLaXQvY2hyb21pdW0vQ2hh bmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTAtMDMtMzEgIEZpbm51 ciBUaG9yYXJpbnNzb24gIDxmaW5udXIud2Via2l0QGdtYWlsLmNvbT4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbY2hyb21pdW1dIEZpbmRJblBhZ2Ug c2hvdWxkIGNsZWFyIHRoZSBmb2N1c2VkIG5vZGUgd2hlbiBhIG1hdGNoIGhhcworICAgICAgICBi ZWVuIGZvdW5kLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0zNjkyMworCisgICAgICAgICogc3JjL1dlYkZyYW1lSW1wbC5jcHA6CisgICAgICAgIChX ZWJLaXQ6OldlYkZyYW1lSW1wbDo6ZmluZCk6CisKIDIwMTAtMDMtMzEgIEFscGhhIExhbSAgPGhj bGFtQGNocm9taXVtLm9yZz4KIAogICAgICAgICBOb3QgcmV2aWV3ZWQuIEJ1aWxkIGZpeC4KSW5k ZXg6IFdlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmNwcAo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBX ZWJLaXQvY2hyb21pdW0vc3JjL1dlYkZyYW1lSW1wbC5jcHAJKHJldmlzaW9uIDU2ODc1KQorKysg V2ViS2l0L2Nocm9taXVtL3NyYy9XZWJGcmFtZUltcGwuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0x MzIzLDYgKzEzMjMsOSBAQCBib29sIFdlYkZyYW1lSW1wbDo6ZmluZChpbnQgaWRlbnRpZmllciwK ICAgICAgICAgICAgIGV4ZWN1dGVDb21tYW5kKFdlYlN0cmluZzo6ZnJvbVVURjgoIlVuc2VsZWN0 IikpOwogICAgICAgICB9CiAKKyAgICAgICAgLy8gTWFrZSBzdXJlIG5vIG5vZGUgaXMgZm9jdXNl ZC4gU2VlIGh0dHA6Ly9jcmJ1Zy5jb20vMzg3MDAuCisgICAgICAgIGZyYW1lKCktPmRvY3VtZW50 KCktPnNldEZvY3VzZWROb2RlKE5VTEwpOworCiAgICAgICAgIGlmICghb3B0aW9ucy5maW5kTmV4 dCB8fCBhY3RpdmVTZWxlY3Rpb24pIHsKICAgICAgICAgICAgIC8vIFRoaXMgaXMgZWl0aGVyIGEg RmluZCBvcGVyYXRpb24gb3IgYSBGaW5kLW5leHQgZnJvbSBhIG5ldyBzdGFydCBwb2ludAogICAg ICAgICAgICAgLy8gZHVlIHRvIGEgc2VsZWN0aW9uLCBzbyB3ZSBzZXQgdGhlIGZsYWcgdG8gYXNr IHRoZSBzY29waW5nIGVmZm9ydAo= 52245 2010-03-31 20:01:48 -0700 2010-04-01 09:13:56 -0700 Now with style issue fixed find_focus_fix.patch text/plain 1328 finnur.webkit SW5kZXg6IFdlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViS2l0L2No cm9taXVtL0NoYW5nZUxvZwkocmV2aXNpb24gNTY4ODYpCisrKyBXZWJLaXQvY2hyb21pdW0vQ2hh bmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTAtMDMtMzEgIEZpbm51 ciBUaG9yYXJpbnNzb24gIDxmaW5udXIud2Via2l0QGdtYWlsLmNvbT4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbY2hyb21pdW1dIEZpbmRJblBhZ2Ug c2hvdWxkIGNsZWFyIHRoZSBmb2N1c2VkIG5vZGUgd2hlbiBhIG1hdGNoIGhhcworICAgICAgICBi ZWVuIGZvdW5kLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0zNjkyMworCisgICAgICAgICogc3JjL1dlYkZyYW1lSW1wbC5jcHA6CisgICAgICAgIChX ZWJLaXQ6OldlYkZyYW1lSW1wbDo6ZmluZCk6CisKIDIwMTAtMDMtMzEgIEFscGhhIExhbSAgPGhj bGFtQGNocm9taXVtLm9yZz4KIAogICAgICAgICBOb3QgcmV2aWV3ZWQuIEJ1aWxkIGZpeC4KSW5k ZXg6IFdlYktpdC9jaHJvbWl1bS9zcmMvV2ViRnJhbWVJbXBsLmNwcAo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBX ZWJLaXQvY2hyb21pdW0vc3JjL1dlYkZyYW1lSW1wbC5jcHAJKHJldmlzaW9uIDU2ODc1KQorKysg V2ViS2l0L2Nocm9taXVtL3NyYy9XZWJGcmFtZUltcGwuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0x MzIzLDYgKzEzMjMsOSBAQCBib29sIFdlYkZyYW1lSW1wbDo6ZmluZChpbnQgaWRlbnRpZmllciwK ICAgICAgICAgICAgIGV4ZWN1dGVDb21tYW5kKFdlYlN0cmluZzo6ZnJvbVVURjgoIlVuc2VsZWN0 IikpOwogICAgICAgICB9CiAKKyAgICAgICAgLy8gTWFrZSBzdXJlIG5vIG5vZGUgaXMgZm9jdXNl ZC4gU2VlIGh0dHA6Ly9jcmJ1Zy5jb20vMzg3MDAuCisgICAgICAgIGZyYW1lKCktPmRvY3VtZW50 KCktPnNldEZvY3VzZWROb2RlKDApOworCiAgICAgICAgIGlmICghb3B0aW9ucy5maW5kTmV4dCB8 fCBhY3RpdmVTZWxlY3Rpb24pIHsKICAgICAgICAgICAgIC8vIFRoaXMgaXMgZWl0aGVyIGEgRmlu ZCBvcGVyYXRpb24gb3IgYSBGaW5kLW5leHQgZnJvbSBhIG5ldyBzdGFydCBwb2ludAogICAgICAg ICAgICAgLy8gZHVlIHRvIGEgc2VsZWN0aW9uLCBzbyB3ZSBzZXQgdGhlIGZsYWcgdG8gYXNrIHRo ZSBzY29waW5nIGVmZm9ydAo= 52303 2010-04-01 09:13:56 -0700 2010-04-01 09:45:51 -0700 Updated description find_focus_fix.patch text/plain 1481 finnur.webkit SW5kZXg6IFdlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViS2l0L2No cm9taXVtL0NoYW5nZUxvZwkocmV2aXNpb24gNTY4ODYpCisrKyBXZWJLaXQvY2hyb21pdW0vQ2hh bmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTAtMDMtMzEgIEZpbm51 ciBUaG9yYXJpbnNzb24gIDxmaW5udXIud2Via2l0QGdtYWlsLmNvbT4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbY2hyb21pdW1dIEZpbmRJblBhZ2Ug c2hvdWxkIGNsZWFyIHRoZSBmb2N1c2VkIG5vZGUgd2hlbiBhIG1hdGNoIGhhcworICAgICAgICBi ZWVuIGZvdW5kLiBUaGlzIGlzIGJlY2F1c2UgV2ViRnJhbWVJbXBsOjpzZXRGb2N1cyB3aWxsIHRy eSB0byByZWZvY3VzCisgICAgICAgIGVkaXRhYmxlIGVsZW1lbnRzIGlmIGl0IHRoaW5rcyB0aGV5 IGhhdmUgZm9jdXMsIGNhdXNpbmcgdGhlIHBhZ2UgdG8KKyAgICAgICAgc2Nyb2xsLgorCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zNjkyMworCisgICAg ICAgICogc3JjL1dlYkZyYW1lSW1wbC5jcHA6CisgICAgICAgIChXZWJLaXQ6OldlYkZyYW1lSW1w bDo6ZmluZCk6CisKIDIwMTAtMDMtMzEgIEFscGhhIExhbSAgPGhjbGFtQGNocm9taXVtLm9yZz4K IAogICAgICAgICBOb3QgcmV2aWV3ZWQuIEJ1aWxkIGZpeC4KSW5kZXg6IFdlYktpdC9jaHJvbWl1 bS9zcmMvV2ViRnJhbWVJbXBsLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXQvY2hyb21pdW0vc3Jj L1dlYkZyYW1lSW1wbC5jcHAJKHJldmlzaW9uIDU2ODc1KQorKysgV2ViS2l0L2Nocm9taXVtL3Ny Yy9XZWJGcmFtZUltcGwuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xMzIzLDYgKzEzMjMsOSBAQCBi b29sIFdlYkZyYW1lSW1wbDo6ZmluZChpbnQgaWRlbnRpZmllciwKICAgICAgICAgICAgIGV4ZWN1 dGVDb21tYW5kKFdlYlN0cmluZzo6ZnJvbVVURjgoIlVuc2VsZWN0IikpOwogICAgICAgICB9CiAK KyAgICAgICAgLy8gTWFrZSBzdXJlIG5vIG5vZGUgaXMgZm9jdXNlZC4gU2VlIGh0dHA6Ly9jcmJ1 Zy5jb20vMzg3MDAuCisgICAgICAgIGZyYW1lKCktPmRvY3VtZW50KCktPnNldEZvY3VzZWROb2Rl KDApOworCiAgICAgICAgIGlmICghb3B0aW9ucy5maW5kTmV4dCB8fCBhY3RpdmVTZWxlY3Rpb24p IHsKICAgICAgICAgICAgIC8vIFRoaXMgaXMgZWl0aGVyIGEgRmluZCBvcGVyYXRpb24gb3IgYSBG aW5kLW5leHQgZnJvbSBhIG5ldyBzdGFydCBwb2ludAogICAgICAgICAgICAgLy8gZHVlIHRvIGEg c2VsZWN0aW9uLCBzbyB3ZSBzZXQgdGhlIGZsYWcgdG8gYXNrIHRoZSBzY29waW5nIGVmZm9ydAo=