Merge updateSelectionCachesIfSelectionIsInsideTextFormControl into setSelectionWithou...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Feb 2014 05:27:17 +0000 (05:27 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 8 Feb 2014 05:27:17 +0000 (05:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=128439

Reviewed by Anders Carlsson.

FrameSelection::selectAll had a superfluous call to updateSelectionCachesIfSelectionIsInsideTextFormControl
because it wasn't setting UserTriggered option to avoid revealing selection.

Call setSelection with UserTriggered and recently added DoNotRevealSelection option and merge
updateSelectionCachesIfSelectionIsInsideTextFormControl into setSelectionWithoutUpdatingAppearance.

Also rename local variables in setSelectionWithoutUpdatingAppearance, newSelection to
newSelectionPossiblyWithoutDirection and s to newSelection so that they're self explanatory.

In addition, we now update the input element's selection caches before calling
selectFrameElementInParentIfFullySelected but this should be fine because selection cannot simultaneously
select the entire document and be inside a text form control.

* editing/FrameSelection.cpp:
(WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance):
(WebCore::FrameSelection::selectAll):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@163692 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/editing/FrameSelection.cpp

index f5560a60fe152ac9b2417f033180b9c126f112f7..b7bfa0f7a5701f711accaa294d1fb00f65560de3 100644 (file)
@@ -1,3 +1,27 @@
+2014-02-07  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Merge updateSelectionCachesIfSelectionIsInsideTextFormControl into setSelectionWithoutUpdatingAppearance
+        https://bugs.webkit.org/show_bug.cgi?id=128439
+
+        Reviewed by Anders Carlsson.
+
+        FrameSelection::selectAll had a superfluous call to updateSelectionCachesIfSelectionIsInsideTextFormControl
+        because it wasn't setting UserTriggered option to avoid revealing selection.
+
+        Call setSelection with UserTriggered and recently added DoNotRevealSelection option and merge
+        updateSelectionCachesIfSelectionIsInsideTextFormControl into setSelectionWithoutUpdatingAppearance.
+
+        Also rename local variables in setSelectionWithoutUpdatingAppearance, newSelection to
+        newSelectionPossiblyWithoutDirection and s to newSelection so that they're self explanatory.
+
+        In addition, we now update the input element's selection caches before calling
+        selectFrameElementInParentIfFullySelected but this should be fine because selection cannot simultaneously
+        select the entire document and be inside a text form control.
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance):
+        (WebCore::FrameSelection::selectAll):
+
 2014-02-07  Ryosuke Niwa  <rniwa@webkit.org>
 
         EFL build fix attempt after r163662.
index 8427e78f4d6d44a23996f407a0c094eb9ef1c446..b28e7c55e117c604144e733a1bf482a508f1a14b 100644 (file)
@@ -250,33 +250,33 @@ void FrameSelection::setSelectionByMouseIfDifferent(const VisibleSelection& pass
     setSelection(newSelection, UserTriggered | DoNotRevealSelection | CloseTyping | ClearTypingStyle, AlignCursorOnScrollIfNeeded, granularity);
 }
 
-bool FrameSelection::setSelectionWithoutUpdatingAppearance(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity, EUserTriggered userTriggered)
+bool FrameSelection::setSelectionWithoutUpdatingAppearance(const VisibleSelection& newSelectionPossiblyWithoutDirection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity, EUserTriggered userTriggered)
 {
     bool closeTyping = options & CloseTyping;
     bool shouldClearTypingStyle = options & ClearTypingStyle;
 
-    VisibleSelection s = newSelection;
+    VisibleSelection newSelection = newSelectionPossiblyWithoutDirection;
     if (shouldAlwaysUseDirectionalSelection(m_frame))
-        s.setIsDirectional(true);
+        newSelection.setIsDirectional(true);
 
     if (!m_frame) {
-        m_selection = s;
+        m_selection = newSelection;
         return false;
     }
 
     // <http://bugs.webkit.org/show_bug.cgi?id=23464>: Infinite recursion at FrameSelection::setSelection
     // if document->frame() == m_frame we can get into an infinite loop
-    if (s.base().anchorNode()) {
-        Document& document = s.base().anchorNode()->document();
-        if (document.frame() && document.frame() != m_frame && &document != m_frame->document()) {
-            RefPtr<Frame> guard = document.frame();
-            document.frame()->selection().setSelection(s, options, align, granularity);
-            // It's possible that during the above set selection, this FrameSelection has been modified by
-            // selectFrameElementInParentIfFullySelected, but that the selection is no longer valid since
-            // the frame is about to be destroyed. If this is the case, clear our selection.
-            if (guard->hasOneRef() && !m_selection.isNonOrphanedCaretOrRange())
-                clear();
-            return false;
+    if (Document* newSelectionDocument = newSelection.base().document()) {
+        if (RefPtr<Frame> newSelectionFrame = newSelectionDocument->frame()) {
+            if (newSelectionFrame != m_frame && newSelectionDocument != m_frame->document()) {
+                newSelectionFrame->selection().setSelection(newSelection, options, align, granularity);
+                // It's possible that during the above set selection, this FrameSelection has been modified by
+                // selectFrameElementInParentIfFullySelected, but that the selection is no longer valid since
+                // the frame is about to be destroyed. If this is the case, clear our selection.
+                if (newSelectionFrame->hasOneRef() && !m_selection.isNonOrphanedCaretOrRange())
+                    clear();
+                return false;
+            }
         }
     }
 
@@ -292,25 +292,25 @@ bool FrameSelection::setSelectionWithoutUpdatingAppearance(const VisibleSelectio
     if (shouldClearTypingStyle)
         clearTypingStyle();
 
-    if (m_selection == s) {
-        // Even if selection was not changed, selection offsets may have been changed.
-        updateSelectionCachesIfSelectionIsInsideTextFormControl(userTriggered);
-        return false;
-    }
-
     VisibleSelection oldSelection = m_selection;
+    m_selection = newSelection;
+
+    // Selection offsets should increase when LF is inserted before the caret in InsertLineBreakCommand. See <https://webkit.org/b/56061>.
+    if (HTMLTextFormControlElement* textControl = enclosingTextFormControl(newSelection.start()))
+        textControl->selectionChanged(userTriggered == UserTriggered);
+
+    if (oldSelection == newSelection)
+        return false;
 
-    m_selection = s;
     setCaretRectNeedsUpdate();
-    
-    if (!s.isNone() && !(options & DoNotSetFocus))
+
+    if (!newSelection.isNone() && !(options & DoNotSetFocus))
         setFocusedElementIfNeeded();
 
     // Always clear the x position used for vertical arrow navigation.
     // It will be restored by the vertical arrow navigation code if necessary.
     m_xPosForVerticalArrowNavigation = NoXPosForVerticalArrowNavigation();
     selectFrameElementInParentIfFullySelected();
-    updateSelectionCachesIfSelectionIsInsideTextFormControl(userTriggered);
     m_frame->editor().respondToChangedSelection(oldSelection, options);
     m_frame->document()->enqueueDocumentEvent(Event::create(eventNames().selectionchangeEvent, false, false));
 
@@ -1706,10 +1706,7 @@ void FrameSelection::selectAll()
     VisibleSelection newSelection(VisibleSelection::selectionFromContentsOfNode(root.get()));
 
     if (shouldChangeSelection(newSelection))
-        setSelection(newSelection);
-
-    selectFrameElementInParentIfFullySelected();
-    updateSelectionCachesIfSelectionIsInsideTextFormControl(UserTriggered);
+        setSelection(newSelection, UserTriggered | CloseTyping | ClearTypingStyle | DoNotRevealSelection);
 }
 
 bool FrameSelection::setSelectedRange(Range* range, EAffinity affinity, bool closeTyping)
@@ -1912,12 +1909,6 @@ void FrameSelection::caretBlinkTimerFired(Timer<FrameSelection>&)
 #endif
 }
 
-void FrameSelection::updateSelectionCachesIfSelectionIsInsideTextFormControl(EUserTriggered userTriggered)
-{
-    if (HTMLTextFormControlElement* textControl = enclosingTextFormControl(m_selection.start()))
-        textControl->selectionChanged(userTriggered == UserTriggered);
-}
-
 // Helper function that tells whether a particular node is an element that has an entire
 // Frame and FrameView, a <frame>, <iframe>, or <object>.
 static bool isFrameElement(const Node* n)