Chromium has some issues where the Find in page is not always starting from the right point after you close the Find session. This is described in this bug: http://code.google.com/p/chromium/issues/detail?id=27910 Basically, a fix was put in for a nasty focus issue for Gmail where we set the selection to 0,0. This is supposed to be done for content-editable fields only, but was not restricted to content-editable fields. Now it is. We otherwise end up setting the selection to 0,0 more often than needed, which can in certain scenarios confuse the find algorithm into thinking the user messed with the selection during runs. On a related note, I also noticed that Find isn't setting the selection when we end the Find session anymore (will follow this patch up with a browser test to catch this in the future).
Created attachment 44435 [details] Fix for Chromium issue http://crbug.com/27910
style-queue ran check-webkit-style on attachment 44435 [details] without any errors.
Comment on attachment 44435 [details] Fix for Chromium issue http://crbug.com/27910 > Index: WebKit/chromium/src/WebFrameImpl.cpp > =================================================================== > --- WebKit/chromium/src/WebFrameImpl.cpp (revision 51798) > +++ WebKit/chromium/src/WebFrameImpl.cpp (working copy) > @@ -1669,6 +1669,7 @@ void WebFrameImpl::setFindEndstateFocusA > + // No node related to the active match was focusable, so set the > + // active match as the selection (so that when you end the Find session, > + // you'll have the last thing you found highlighted) and make sure that > + // we have nothing focused (otherwise you might have text selected but > + // a link focused, which is weird). > + frame()->selection()->setSelection(m_activeMatch.get()); > + frame()->document()->setFocusedNode(0); Doesn't setSelection already set the focus appropriately? e.g. if you set the selection to some text inside a link, shouldn't the link be focused?
The intent here is to have the whole link appear with the focus ring around it (consistent with Firefox) and be able to press Enter to navigate to that url. If I just do the selection part, and search for a partial text match within a link, then that partial match gets highlighted when I close the box, no focus ring appears and pressing Enter does nothing.
Comment on attachment 44435 [details] Fix for Chromium issue http://crbug.com/27910 Since the WebKit bug links to the Chromium bug, I think it is sufficient to just include the WebKit bug link in the ChangeLog. > Index: WebKit/chromium/src/WebFrameImpl.cpp ... > if (node && node != frame()->document()) { > // Found a focusable parent node. Set focus to it. > frame()->document()->setFocusedNode(node); > + return; > } else { nit: now that have the return statement, please remove the else statement so that you can reduce the indentation of the following code. R=me otherwise.
Created attachment 44443 [details] Addressed review comments
Comment on attachment 44443 [details] Addressed review comments Clearing flags on attachment: 44443 Committed r51818: <http://trac.webkit.org/changeset/51818>
All reviewed patches have been landed. Closing bug.