Modified: trunk/Source/WebCore/ChangeLog (163688 => 163689)
--- trunk/Source/WebCore/ChangeLog 2014-02-08 03:11:53 UTC (rev 163688)
+++ trunk/Source/WebCore/ChangeLog 2014-02-08 03:50:22 UTC (rev 163689)
@@ -1,3 +1,27 @@
+2014-02-07 Ryosuke Niwa <rn...@webkit.org>
+
+ FrameSelection's destructor shouldn't notify accessibility
+ https://bugs.webkit.org/show_bug.cgi?id=128421
+
+ Reviewed by Benjamin Poulain.
+
+ 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 Martin Robinson <mrobin...@igalia.com>
Build fix for the GTK+ CMake build
Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (163688 => 163689)
--- trunk/Source/WebCore/editing/FrameSelection.cpp 2014-02-08 03:11:53 UTC (rev 163688)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp 2014-02-08 03:50:22 UTC (rev 163689)
@@ -250,11 +250,10 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
if (view)
view->clearSelection();
- setSelection(VisibleSelection(), CloseTyping | ClearTypingStyle | DoNotUpdateAppearance);
+ setSelectionWithoutUpdatingAppearance(VisibleSelection(), CloseTyping | ClearTypingStyle,
+ AlignCursorOnScrollIfNeeded, CharacterGranularity, NotUserTriggered);
m_previousCaretNode.clear();
}
Modified: trunk/Source/WebCore/editing/FrameSelection.h (163688 => 163689)
--- trunk/Source/WebCore/editing/FrameSelection.h 2014-02-08 03:11:53 UTC (rev 163688)
+++ trunk/Source/WebCore/editing/FrameSelection.h 2014-02-08 03:50:22 UTC (rev 163689)
@@ -124,8 +124,7 @@
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 @@
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 @@
#if HAVE(ACCESSIBILITY)
void notifyAccessibilityForSelectionChange();
+#else
+ void notifyAccessibilityForSelectionChange() { }
#endif
void updateSelectionCachesIfSelectionIsInsideTextFormControl(EUserTriggered);
