<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[163689] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/163689">163689</a></dd>
<dt>Author</dt> <dd>rniwa@webkit.org</dd>
<dt>Date</dt> <dd>2014-02-07 19:50:22 -0800 (Fri, 07 Feb 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreeditingFrameSelectioncpp">trunk/Source/WebCore/editing/FrameSelection.cpp</a></li>
<li><a href="#trunkSourceWebCoreeditingFrameSelectionh">trunk/Source/WebCore/editing/FrameSelection.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (163688 => 163689)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+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 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.
+
</ins><span class="cx"> 2014-02-07 Martin Robinson <mrobinson@igalia.com>
</span><span class="cx">
</span><span class="cx"> Build fix for the GTK+ CMake build
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingFrameSelectioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (163688 => 163689)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -250,11 +250,10 @@
</span><span class="cx"> setSelection(newSelection, UserTriggered | DoNotRevealSelection | CloseTyping | ClearTypingStyle, AlignCursorOnScrollIfNeeded, granularity);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity)
</del><ins>+bool FrameSelection::setSelectionWithoutUpdatingAppearance(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity, EUserTriggered userTriggered)
</ins><span class="cx"> {
</span><span class="cx"> bool closeTyping = options & CloseTyping;
</span><span class="cx"> bool shouldClearTypingStyle = options & ClearTypingStyle;
</span><del>- EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);
</del><span class="cx">
</span><span class="cx"> VisibleSelection s = newSelection;
</span><span class="cx"> if (shouldAlwaysUseDirectionalSelection(m_frame))
</span><span class="lines">@@ -262,7 +261,7 @@
</span><span class="cx">
</span><span class="cx"> if (!m_frame) {
</span><span class="cx"> m_selection = s;
</span><del>- return;
</del><ins>+ return false;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // <http://bugs.webkit.org/show_bug.cgi?id=23464>: Infinite recursion at FrameSelection::setSelection
</span><span class="lines">@@ -277,7 +276,7 @@
</span><span class="cx"> // the frame is about to be destroyed. If this is the case, clear our selection.
</span><span class="cx"> if (guard->hasOneRef() && !m_selection.isNonOrphanedCaretOrRange())
</span><span class="cx"> clear();
</span><del>- return;
</del><ins>+ return false;
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -296,7 +295,7 @@
</span><span class="cx"> if (m_selection == s) {
</span><span class="cx"> // Even if selection was not changed, selection offsets may have been changed.
</span><span class="cx"> updateSelectionCachesIfSelectionIsInsideTextFormControl(userTriggered);
</span><del>- return;
</del><ins>+ return false;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> VisibleSelection oldSelection = m_selection;
</span><span class="lines">@@ -307,21 +306,30 @@
</span><span class="cx"> if (!s.isNone() && !(options & DoNotSetFocus))
</span><span class="cx"> setFocusedElementIfNeeded();
</span><span class="cx">
</span><del>- if (!(options & DoNotUpdateAppearance)) {
-#if ENABLE(TEXT_CARET)
- m_frame->document()->updateLayoutIgnorePendingStylesheets();
-#else
- m_frame->document()->updateStyleIfNeeded();
-#endif
- updateAppearance();
- }
-
</del><span class="cx"> // Always clear the x position used for vertical arrow navigation.
</span><span class="cx"> // It will be restored by the vertical arrow navigation code if necessary.
</span><span class="cx"> m_xPosForVerticalArrowNavigation = NoXPosForVerticalArrowNavigation();
</span><span class="cx"> selectFrameElementInParentIfFullySelected();
</span><span class="cx"> updateSelectionCachesIfSelectionIsInsideTextFormControl(userTriggered);
</span><span class="cx"> m_frame->editor().respondToChangedSelection(oldSelection, options);
</span><ins>+ 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();
+
</ins><span class="cx"> if (userTriggered == UserTriggered && !(options & DoNotRevealSelection)) {
</span><span class="cx"> ScrollAlignment alignment;
</span><span class="cx">
</span><span class="lines">@@ -332,10 +340,8 @@
</span><span class="cx">
</span><span class="cx"> revealSelection(alignment, RevealExtent);
</span><span class="cx"> }
</span><del>-#if HAVE(ACCESSIBILITY)
</del><ins>+
</ins><span class="cx"> notifyAccessibilityForSelectionChange();
</span><del>-#endif
- m_frame->document()->enqueueDocumentEvent(Event::create(eventNames().selectionchangeEvent, false, false));
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> static bool removingNodeRemovesPosition(Node* node, const Position& position)
</span><span class="lines">@@ -1237,7 +1243,8 @@
</span><span class="cx"> if (view)
</span><span class="cx"> view->clearSelection();
</span><span class="cx">
</span><del>- setSelection(VisibleSelection(), CloseTyping | ClearTypingStyle | DoNotUpdateAppearance);
</del><ins>+ setSelectionWithoutUpdatingAppearance(VisibleSelection(), CloseTyping | ClearTypingStyle,
+ AlignCursorOnScrollIfNeeded, CharacterGranularity, NotUserTriggered);
</ins><span class="cx"> m_previousCaretNode.clear();
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingFrameSelectionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/FrameSelection.h (163688 => 163689)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -124,8 +124,7 @@
</span><span class="cx"> SpellCorrectionTriggered = 1 << 3,
</span><span class="cx"> DoNotSetFocus = 1 << 4,
</span><span class="cx"> DictationTriggered = 1 << 5,
</span><del>- DoNotUpdateAppearance = 1 << 6,
- DoNotRevealSelection = 1 << 7,
</del><ins>+ DoNotRevealSelection = 1 << 6,
</ins><span class="cx"> };
</span><span class="cx"> typedef unsigned SetSelectionOptions; // Union of values in SetSelectionOption and EUserTriggered
</span><span class="cx"> static inline EUserTriggered selectionOptionsToUserTriggered(SetSelectionOptions options)
</span><span class="lines">@@ -280,6 +279,8 @@
</span><span class="cx"> private:
</span><span class="cx"> enum EPositionType { START, END, BASE, EXTENT };
</span><span class="cx">
</span><ins>+ bool setSelectionWithoutUpdatingAppearance(const VisibleSelection&, SetSelectionOptions, CursorAlignOnScroll, TextGranularity, EUserTriggered);
+
</ins><span class="cx"> void respondToNodeModification(Node*, bool baseRemoved, bool extentRemoved, bool startRemoved, bool endRemoved);
</span><span class="cx"> TextDirection directionOfEnclosingBlock();
</span><span class="cx"> TextDirection directionOfSelection();
</span><span class="lines">@@ -302,6 +303,8 @@
</span><span class="cx">
</span><span class="cx"> #if HAVE(ACCESSIBILITY)
</span><span class="cx"> void notifyAccessibilityForSelectionChange();
</span><ins>+#else
+ void notifyAccessibilityForSelectionChange() { }
</ins><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> void updateSelectionCachesIfSelectionIsInsideTextFormControl(EUserTriggered);
</span></span></pre>
</div>
</div>
</body>
</html>