Bug 36923 – [Chromium]: FindInPage issue with focus remaining on element after match is found

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Finnur Thorarinsson]

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.

[Finnur Thorarinsson]

Created attachment 52243 [details]
Patch for Find issue 36923

[WebKit Review Bot]

Attachment 52243 [details] 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.

[Finnur Thorarinsson]

Created attachment 52245 [details]
Now with style issue fixed

[Ojan Vafai]

Comment on attachment 52245 [details]
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 @@
> [phone]  Finnur Thorarinsson  <[e-mail]>
> +
> +        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.

[Finnur Thorarinsson]

Created attachment 52303 [details]
Updated description

[Dimitri Glazkov (Google)]

Comment on attachment 52303 [details]
Updated description

ok.

[WebKit Commit Bot]

Comment on attachment 52303 [details]
Updated description

Clearing flags on attachment: 52303

Committed r56917: <http://trac.webkit.org/changeset/56917>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.