Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(776)

Unified Diff: Source/core/editing/FrameSelection.cpp

Issue 988023005: Implementing directional selection strategy in Blink. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Addressing feedback. Created 5 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/editing/FrameSelection.cpp
diff --git a/Source/core/editing/FrameSelection.cpp b/Source/core/editing/FrameSelection.cpp
index 0cb682f643ce648cd3916e4093ffbb688e47085f..d93e0dc1fe4c1e98b893e7b8de909f2e5f45ab0a 100644
--- a/Source/core/editing/FrameSelection.cpp
+++ b/Source/core/editing/FrameSelection.cpp
@@ -38,6 +38,7 @@
#include "core/dom/NodeTraversal.h"
#include "core/dom/Text.h"
#include "core/editing/Editor.h"
+#include "core/editing/GranularityStrategy.h"
#include "core/editing/InputMethodController.h"
#include "core/editing/RenderedPosition.h"
#include "core/editing/SpellChecker.h"
@@ -222,11 +223,15 @@ void FrameSelection::setNonDirectionalSelectionIfNeeded(const VisibleSelection&
void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity)
{
+ if (m_granularityStrategy && (options & FrameSelection::DoNotClearStrategy) == 0) {
yosin_UTC9 2015/04/23 03:38:47 nit: We don't need to have "{}".
mfomitchev 2015/04/23 15:08:11 Done.
+ m_granularityStrategy->Clear();
+ }
bool closeTyping = options & CloseTyping;
bool shouldClearTypingStyle = options & ClearTypingStyle;
EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);
VisibleSelection s = validateSelection(newSelection);
+
yosin_UTC9 2015/04/23 03:38:47 nit: No need to have an extra blank line.
mfomitchev 2015/04/23 15:08:11 Done.
if (shouldAlwaysUseDirectionalSelection(m_frame))
s.setIsDirectional(true);
@@ -1173,6 +1178,8 @@ LayoutUnit FrameSelection::lineDirectionPointForBlockDirectionNavigation(EPositi
void FrameSelection::clear()
{
m_granularity = CharacterGranularity;
+ if (m_granularityStrategy)
+ m_granularityStrategy->Clear();
setSelection(VisibleSelection());
}
@@ -1913,21 +1920,38 @@ bool FrameSelection::selectWordAroundPosition(const VisiblePosition& position)
return false;
}
-void FrameSelection::moveRangeSelectionExtent(const VisiblePosition& extentPosition, TextGranularity granularity)
+GranularityStrategy* FrameSelection::granularityStrategy()
{
- if (isNone())
- return;
+ // We do lazy initalization for m_granularityStrategy, because if we initialize it
+ // right in the constructor - the correct settings may not be set yet.
+ SelectionStrategy strategyType = SelectionStrategy::Character;
+ Settings* settings = m_frame ? m_frame->settings() : 0;
+ if (settings && settings->selectionStrategy() == SelectionStrategy::Direction)
+ strategyType = SelectionStrategy::Direction;
- const VisiblePosition basePosition = m_selection.isBaseFirst() ? m_selection.visibleStart() : m_selection.visibleEnd();
- VisibleSelection newSelection(basePosition, extentPosition);
- if (newSelection.isBaseFirst())
- newSelection.setEndRespectingGranularity(granularity);
+ if (m_granularityStrategy && m_granularityStrategy->GetType() == strategyType)
+ return m_granularityStrategy.get();
+
+ if (strategyType == SelectionStrategy::Direction)
+ m_granularityStrategy = adoptPtr(new DirectionGranularityStrategy());
else
- newSelection.setStartRespectingGranularity(granularity);
- if (!newSelection.isRange())
+ m_granularityStrategy = adoptPtr(new CharacterGranularityStrategy());
+ return m_granularityStrategy.get();
+}
+
+void FrameSelection::moveRangeSelectionExtent(const VisiblePosition& extentPosition)
+{
+ const VisiblePosition base = m_selection.visibleBase();
yosin_UTC9 2015/04/23 03:38:47 nit: We don't need to have |const|.
mfomitchev 2015/04/23 15:08:11 Done.
+
+ if (isNone() || base == extentPosition)
yosin_UTC9 2015/04/23 03:38:47 nit: Can we test |isNone()| before calling |Visibl
mfomitchev 2015/04/23 15:08:11 Done.
return;
- setSelection(newSelection, FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle | UserTriggered, FrameSelection::AlignCursorOnScrollIfNeeded, granularity);
+ VisibleSelection newSelection = granularityStrategy()->updateExtent(extentPosition, selection());
+ setSelection(
+ newSelection,
+ FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle | FrameSelection::DoNotClearStrategy | UserTriggered,
+ FrameSelection::AlignCursorOnScrollIfNeeded,
+ CharacterGranularity);
}
void FrameSelection::moveRangeSelection(const VisiblePosition& basePosition, const VisiblePosition& extentPosition, TextGranularity granularity)

Powered by Google App Engine
This is Rietveld 408576698