Attachment 223539 Details for Bug 128421 – Reverted chagnes to xcscheme

[patch] Reverted chagnes to xcscheme

bug-128421-20140207183907.patch (text/plain), 7.89 KB, created by Ryosuke Niwa on 2014-02-07 18:39:07 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2014-02-07 18:39:07 PST

Size: 7.89 KB

patch

obsolete

>Subversion Revision: 163659
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e4abcbaa9b0d0ec3dd21c9e7e5adf52a4054c302..3d5a9ca3c657878a91011085d6bb4db032976498 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2014-02-07  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        FrameSelection's destructor shouldn't notify accessibility
>+        https://bugs.webkit.org/show_bug.cgi?id=128421
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Extracted setSelectionWithoutUpdatingAppearance out of setSelection and called it in prepareForDestruction
>+        instead of setting DoNotUpdateAppearance option. This new function doesn't reveal selection or notify
>+        accessibility code in addition to not updating appearance.
>+
>+        Note that all implementations of notifyAccessibilityForSelectionChange in FrameSelectionAtk.cpp and
>+        FrameSelectionMac.mm exit early when the selection type is not caret or either start or end is null,
>+        which is already the case inside FrameSelection's destructor. In addition, revealSelection is never called
>+        when selection change was not triggered by user so there should be no behavioral change from this patch.
>+
>+        * editing/FrameSelection.cpp:
>+        (WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance):
>+        (WebCore::FrameSelection::setSelection):
>+        (WebCore::FrameSelection::prepareForDestruction):
>+        * editing/FrameSelection.h:
>+        (WebCore::FrameSelection::notifyAccessibilityForSelectionChange): Added the trivial implementation in the case
>+        accessibility is disabled to tidy up call sites.
>+
> 2014-02-07  Roger Fong  <roger_fong@apple.com>
> 
>         CGContextGetUserSpaceToDeviceSpaceTransform returns the wrong value on Windows.
>diff --git a/Source/WebCore/editing/FrameSelection.cpp b/Source/WebCore/editing/FrameSelection.cpp
>index dd68f177aa274e953d82e9cb0a8af26b5f02d460..8427e78f4d6d44a23996f407a0c094eb9ef1c446 100644
>--- a/Source/WebCore/editing/FrameSelection.cpp
>+++ b/Source/WebCore/editing/FrameSelection.cpp
>@@ -250,11 +250,10 @@ void FrameSelection::setSelectionByMouseIfDifferent(const VisibleSelection& pass
>     setSelection(newSelection, UserTriggered | DoNotRevealSelection | CloseTyping | ClearTypingStyle, AlignCursorOnScrollIfNeeded, granularity);
> }
> 
>-void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity)
>+bool FrameSelection::setSelectionWithoutUpdatingAppearance(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity, EUserTriggered userTriggered)
> {
>     bool closeTyping = options & CloseTyping;
>     bool shouldClearTypingStyle = options & ClearTypingStyle;
>-    EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);
> 
>     VisibleSelection s = newSelection;
>     if (shouldAlwaysUseDirectionalSelection(m_frame))
>@@ -262,7 +261,7 @@ void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelec
> 
>     if (!m_frame) {
>         m_selection = s;
>-        return;
>+        return false;
>     }
> 
>     // <http://bugs.webkit.org/show_bug.cgi?id=23464>: Infinite recursion at FrameSelection::setSelection
>@@ -277,7 +276,7 @@ void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelec
>             // the frame is about to be destroyed. If this is the case, clear our selection.
>             if (guard->hasOneRef() && !m_selection.isNonOrphanedCaretOrRange())
>                 clear();
>-            return;
>+            return false;
>         }
>     }
> 
>@@ -296,7 +295,7 @@ void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelec
>     if (m_selection == s) {
>         // Even if selection was not changed, selection offsets may have been changed.
>         updateSelectionCachesIfSelectionIsInsideTextFormControl(userTriggered);
>-        return;
>+        return false;
>     }
> 
>     VisibleSelection oldSelection = m_selection;
>@@ -307,21 +306,30 @@ void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelec
>     if (!s.isNone() && !(options & DoNotSetFocus))
>         setFocusedElementIfNeeded();
> 
>-    if (!(options & DoNotUpdateAppearance)) {
>-#if ENABLE(TEXT_CARET)
>-        m_frame->document()->updateLayoutIgnorePendingStylesheets();
>-#else
>-        m_frame->document()->updateStyleIfNeeded();
>-#endif
>-        updateAppearance();
>-    }
>-
>     // 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));
>+
>+    return true;
>+}
>+
>+void FrameSelection::setSelection(const VisibleSelection& selection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity)
>+{
>+    EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);
>+    if (!setSelectionWithoutUpdatingAppearance(selection, options, align, granularity, userTriggered))
>+        return;
>+
>+#if ENABLE(TEXT_CARET)
>+    m_frame->document()->updateLayoutIgnorePendingStylesheets();
>+#else
>+    m_frame->document()->updateStyleIfNeeded();
>+#endif
>+    updateAppearance();
>+
>     if (userTriggered == UserTriggered && !(options & DoNotRevealSelection)) {
>         ScrollAlignment alignment;
> 
>@@ -332,10 +340,8 @@ void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelec
> 
>         revealSelection(alignment, RevealExtent);
>     }
>-#if HAVE(ACCESSIBILITY)
>+
>     notifyAccessibilityForSelectionChange();
>-#endif
>-    m_frame->document()->enqueueDocumentEvent(Event::create(eventNames().selectionchangeEvent, false, false));
> }
> 
> static bool removingNodeRemovesPosition(Node* node, const Position& position)
>@@ -1237,7 +1243,8 @@ void FrameSelection::prepareForDestruction()
>     if (view)
>         view->clearSelection();
> 
>-    setSelection(VisibleSelection(), CloseTyping | ClearTypingStyle | DoNotUpdateAppearance);
>+    setSelectionWithoutUpdatingAppearance(VisibleSelection(), CloseTyping | ClearTypingStyle,
>+        AlignCursorOnScrollIfNeeded, CharacterGranularity, NotUserTriggered);
>     m_previousCaretNode.clear();
> }
> 
>diff --git a/Source/WebCore/editing/FrameSelection.h b/Source/WebCore/editing/FrameSelection.h
>index 0784fb142e637ca0d7337fa196fe62e204d576c2..f0417a89414847634145252f5c2380c63c1c120d 100644
>--- a/Source/WebCore/editing/FrameSelection.h
>+++ b/Source/WebCore/editing/FrameSelection.h
>@@ -124,8 +124,7 @@ public:
>         SpellCorrectionTriggered = 1 << 3,
>         DoNotSetFocus = 1 << 4,
>         DictationTriggered = 1 << 5,
>-        DoNotUpdateAppearance = 1 << 6,
>-        DoNotRevealSelection = 1 << 7,
>+        DoNotRevealSelection = 1 << 6,
>     };
>     typedef unsigned SetSelectionOptions; // Union of values in SetSelectionOption and EUserTriggered
>     static inline EUserTriggered selectionOptionsToUserTriggered(SetSelectionOptions options)
>@@ -280,6 +279,8 @@ public:
> private:
>     enum EPositionType { START, END, BASE, EXTENT };
> 
>+    bool setSelectionWithoutUpdatingAppearance(const VisibleSelection&, SetSelectionOptions, CursorAlignOnScroll, TextGranularity, EUserTriggered);
>+
>     void respondToNodeModification(Node*, bool baseRemoved, bool extentRemoved, bool startRemoved, bool endRemoved);
>     TextDirection directionOfEnclosingBlock();
>     TextDirection directionOfSelection();
>@@ -302,6 +303,8 @@ private:
> 
> #if HAVE(ACCESSIBILITY)
>     void notifyAccessibilityForSelectionChange();
>+#else
>+    void notifyAccessibilityForSelectionChange() { }
> #endif
> 
>     void updateSelectionCachesIfSelectionIsInsideTextFormControl(EUserTriggered);

Actions: View | Formatted Diff | Diff

Attachments on bug 128421: 223528 | 223538 | 223539