Attachment 223528 Details for Bug 128421 – Cleanup

[patch] Cleanup

bug-128421-20140207174738.patch (text/plain), 6.81 KB, created by Ryosuke Niwa on 2014-02-07 17:47:39 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2014-02-07 17:47:39 PST

Size: 6.81 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..cd1a1ba4f70ef15cf0f3988b5ba4440bf91cd0cd 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)
>+void 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))
>@@ -307,21 +306,29 @@ 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));
>+}
>+
>+void FrameSelection::setSelection(const VisibleSelection& selection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity)
>+{
>+    EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);
>+
>+    setSelectionWithoutUpdatingAppearance(selection, options, align, granularity, userTriggered);
>+
>+#if ENABLE(TEXT_CARET)
>+    m_frame->document()->updateLayoutIgnorePendingStylesheets();
>+#else
>+    m_frame->document()->updateStyleIfNeeded();
>+#endif
>+
>+    updateAppearance();
>+
>     if (userTriggered == UserTriggered && !(options & DoNotRevealSelection)) {
>         ScrollAlignment alignment;
> 
>@@ -332,10 +339,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 +1242,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..cefc99d96f22dcf59dead44c5ef1f5491b0cf8a2 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 };
> 
>+    void 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