Changeset 51818 in webkit

Timestamp:

Dec 7, 2009 5:22:23 PM (14 years ago)

Author:

eric@webkit.org

Message:

2009-12-07 Finnur Thorarinsson <​finnur.webkit@gmail.com>

Reviewed by Darin Fisher.

A patch for Chromium to restrict the scope of the Gmail focus fix,
where we set the Selection to 0,0 for content-editable fields and
also make sure we set the selection end-state for find to select the
text found (when not focusing a link we found).
WebKit bug: ​https://bugs.webkit.org/show_bug.cgi?id=32248

• src/WebFrameImpl.cpp: (WebKit::WebFrameImpl::setFindEndstateFocusAndSelection):
• src/WebViewImpl.cpp: (WebKit::WebViewImpl::setFocus):

Location:

trunk/WebKit/chromium

Files:

• ChangeLog (modified) (1 diff)
• src/WebFrameImpl.cpp (modified) (1 diff)
• src/WebViewImpl.cpp (modified) (1 diff)

trunk/WebKit/chromium/ChangeLog

@@ +1 @@
+2009-12-07  Finnur Thorarinsson  <finnur.webkit@gmail.com>
+
+        Reviewed by Darin Fisher.
+
+        A patch for Chromium to restrict the scope of the Gmail focus fix,
+        where we set the Selection to 0,0 for content-editable fields and
+        also make sure we set the selection end-state for find to select the
+        text found (when not focusing a link we found).
+        WebKit bug: https://bugs.webkit.org/show_bug.cgi?id=32248
+
+        * src/WebFrameImpl.cpp:
+        (WebKit::WebFrameImpl::setFindEndstateFocusAndSelection):
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::setFocus):
+
 2009-12-07  Dmitry Titov  <dimich@chromium.org>
 

trunk/WebKit/chromium/src/WebFrameImpl.cpp

@@ -1670 +1670 @@
             // Found a focusable parent node. Set focus to it.
             frame()->document()->setFocusedNode(node);
-        } else {
-            // Iterate over all the nodes in the range until we find a focusable node.
-            // This, for example, sets focus to the first link if you search for
-            // text and text that is within one or more links.
-            node = m_activeMatch->firstNode();
-            while (node && node != m_activeMatch->pastLastNode()) {
-                if (node->isFocusable()) {
-                    frame()->document()->setFocusedNode(node);
-                    break;
-                }
-                node = node->traverseNextNode();
+            return;
+        }
+
+        // Iterate over all the nodes in the range until we find a focusable node.
+        // This, for example, sets focus to the first link if you search for
+        // text and text that is within one or more links.
+        node = m_activeMatch->firstNode();
+        while (node && node != m_activeMatch->pastLastNode()) {
+            if (node->isFocusable()) {
+                frame()->document()->setFocusedNode(node);
+                return;
             }
+            node = node->traverseNextNode();
         }
+
+        // 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);
     }
 }

trunk/WebKit/chromium/src/WebViewImpl.cpp

@@ -926 +926 @@
                 if (element->isTextFormControl())
                     element->updateFocusAppearance(true);
-                else {
+                else if (focusedNode->isContentEditable()) {
                     // updateFocusAppearance() selects all the text of
                     // contentseditable DIVs. So we set the selection explicitly
                     // instead. Note that this has the side effect of moving the
-                    // caret back to the begining of the text.
+                    // caret back to the beginning of the text.
                     Position position(focusedNode, 0,
                                       Position::PositionIsOffsetInAnchor);