Changeset 89263 in webkit

Timestamp:

Jun 20, 2011 11:28:48 AM (13 years ago)

Author:

rniwa@webkit.org

Message:

2011-06-20 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Ojan Vafai.

FrameSelection::modify should take verticalDisplacement instead of verticalDistance
​https://bugs.webkit.org/show_bug.cgi?id=62932

Added new VerticalDirection enum to the argument list of FrameSelection::modify that takes
verticalDistance. Also changed the type of verticalDistance from int to unsigned int
to accidentally pass a negative distance in the future.

• editing/EditorCommand.cpp: (WebCore::verticalScrollDistance): Returns unsigned int instead of int. (WebCore::executeMovePageDown): Calls FrameSelection::modify. (WebCore::executeMovePageDownAndModifySelection): Ditto. (WebCore::executeMovePageUp): Ditto. (WebCore::executeMovePageUpAndModifySelection): Ditto.
• editing/FrameSelection.cpp: (WebCore::FrameSelection::modify): Takes VerticalDirection as an argument.
• editing/FrameSelection.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• editing/EditorCommand.cpp (modified) (3 diffs)
• editing/FrameSelection.cpp (modified) (7 diffs)
• editing/FrameSelection.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-06-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Ojan Vafai.
+
+        FrameSelection::modify should take verticalDisplacement instead of verticalDistance
+        https://bugs.webkit.org/show_bug.cgi?id=62932
+
+        Added new VerticalDirection enum to the argument list of FrameSelection::modify that takes
+        verticalDistance.  Also changed the type of verticalDistance from int to unsigned int
+        to accidentally pass a negative distance in the future.
+
+        * editing/EditorCommand.cpp:
+        (WebCore::verticalScrollDistance): Returns unsigned int instead of int.
+        (WebCore::executeMovePageDown): Calls FrameSelection::modify.
+        (WebCore::executeMovePageDownAndModifySelection): Ditto.
+        (WebCore::executeMovePageUp): Ditto.
+        (WebCore::executeMovePageUpAndModifySelection): Ditto.
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::modify): Takes VerticalDirection as an argument.
+        * editing/FrameSelection.h:
+
 2011-06-20  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebCore/editing/EditorCommand.cpp

@@ -246 +246 @@
 }
 
-static int verticalScrollDistance(Frame* frame)
+static unsigned verticalScrollDistance(Frame* frame)
 {
     Node* focusedNode = frame->document()->focusedNode();
@@ -259 +259 @@
     if (!(style->overflowY() == OSCROLL || style->overflowY() == OAUTO || focusedNode->rendererIsEditable()))
         return 0;
-    int height = std::min<int>(toRenderBox(renderer)->clientHeight(),
-                               frame->view()->visibleHeight());
-    return max(max<int>(height * Scrollbar::minFractionToStepWhenPaging(), height - Scrollbar::maxOverlapBetweenPages()), 1);
+    int height = std::min<int>(toRenderBox(renderer)->clientHeight(), frame->view()->visibleHeight());
+    return static_cast<unsigned>(max(max<int>(height * Scrollbar::minFractionToStepWhenPaging(), height - Scrollbar::maxOverlapBetweenPages()), 1));
 }
 
@@ -654 +653 @@
 static bool executeMovePageDown(Frame* frame, Event*, EditorCommandSource, const String&)
 {
-    int distance = verticalScrollDistance(frame);
+    unsigned distance = verticalScrollDistance(frame);
     if (!distance)
         return false;
-    return frame->selection()->modify(FrameSelection::AlterationMove, distance, true, FrameSelection::AlignCursorOnScrollAlways);
+    return frame->selection()->modify(FrameSelection::AlterationMove, distance, FrameSelection::DirectionDown, true, FrameSelection::AlignCursorOnScrollAlways);
 }
 
 static bool executeMovePageDownAndModifySelection(Frame* frame, Event*, EditorCommandSource, const String&)
 {
-    int distance = verticalScrollDistance(frame);
+    unsigned distance = verticalScrollDistance(frame);
     if (!distance)
         return false;
-    return frame->selection()->modify(FrameSelection::AlterationExtend, distance, true, FrameSelection::AlignCursorOnScrollAlways);
+    return frame->selection()->modify(FrameSelection::AlterationExtend, distance, FrameSelection::DirectionDown, true, FrameSelection::AlignCursorOnScrollAlways);
 }
 
 static bool executeMovePageUp(Frame* frame, Event*, EditorCommandSource, const String&)
 {
-    int distance = verticalScrollDistance(frame);
+    unsigned distance = verticalScrollDistance(frame);
     if (!distance)
         return false;
-    return frame->selection()->modify(FrameSelection::AlterationMove, -distance, true, FrameSelection::AlignCursorOnScrollAlways);
+    return frame->selection()->modify(FrameSelection::AlterationMove, distance, FrameSelection::DirectionUp, true, FrameSelection::AlignCursorOnScrollAlways);
 }
 
 static bool executeMovePageUpAndModifySelection(Frame* frame, Event*, EditorCommandSource, const String&)
 {
-    int distance = verticalScrollDistance(frame);
+    unsigned distance = verticalScrollDistance(frame);
     if (!distance)
         return false;
-    return frame->selection()->modify(FrameSelection::AlterationExtend, -distance, true, FrameSelection::AlignCursorOnScrollAlways);
+    return frame->selection()->modify(FrameSelection::AlterationExtend, distance, FrameSelection::DirectionUp, true, FrameSelection::AlignCursorOnScrollAlways);
 }
 

trunk/Source/WebCore/editing/FrameSelection.cpp

@@ -889 +889 @@
 }
 
-bool FrameSelection::modify(EAlteration alter, int verticalDistance, bool userTriggered, CursorAlignOnScroll align)
+bool FrameSelection::modify(EAlteration alter, unsigned verticalDistance, VerticalDirection direction, bool userTriggered, CursorAlignOnScroll align)
 {
     if (!verticalDistance)
@@ -898 +898 @@
         trialFrameSelection.setSelection(m_selection);
         trialFrameSelection.setIsDirectional(m_isDirectional);
-        trialFrameSelection.modify(alter, verticalDistance, false);
+        trialFrameSelection.modify(alter, verticalDistance, direction, false);
 
         bool change = shouldChangeSelection(trialFrameSelection.selection());
@@ -905 +905 @@
     }
 
-    bool up = verticalDistance < 0;
-    if (up)
-        verticalDistance = -verticalDistance;
-
-    willBeModified(alter, up ? DirectionBackward : DirectionForward);
+    willBeModified(alter, direction == DirectionUp ? DirectionBackward : DirectionForward);
 
     VisiblePosition pos;
@@ -915 +911 @@
     switch (alter) {
     case AlterationMove:
-        pos = VisiblePosition(up ? m_selection.start() : m_selection.end(), m_selection.affinity());
-        xPos = lineDirectionPointForBlockDirectionNavigation(up ? START : END);
-        m_selection.setAffinity(up ? UPSTREAM : DOWNSTREAM);
+        pos = VisiblePosition(direction == DirectionUp ? m_selection.start() : m_selection.end(), m_selection.affinity());
+        xPos = lineDirectionPointForBlockDirectionNavigation(direction == DirectionUp ? START : END);
+        m_selection.setAffinity(direction == DirectionUp ? UPSTREAM : DOWNSTREAM);
         break;
     case AlterationExtend:
@@ -929 +925 @@
     if (!absoluteCaretY(pos, startY))
         return false;
-    if (up)
+    if (direction == DirectionUp)
         startY = -startY;
     int lastY = startY;
@@ -936 +932 @@
     VisiblePosition next;
     for (VisiblePosition p = pos; ; p = next) {
-        next = (up ? previousLinePosition : nextLinePosition)(p, xPos);
+        next = (direction == DirectionUp ? previousLinePosition : nextLinePosition)(p, xPos);
         if (next.isNull() || next == p)
             break;
@@ -942 +938 @@
         if (!absoluteCaretY(next, nextY))
             break;
-        if (up)
+        if (direction == DirectionUp)
             nextY = -nextY;
-        if (nextY - startY > verticalDistance)
+        if (nextY - startY > static_cast<int>(verticalDistance))
             break;
         if (nextY >= lastY) {

trunk/Source/WebCore/editing/FrameSelection.h

@@ -147 +147 @@
 
     bool modify(EAlteration, SelectionDirection, TextGranularity, bool userTriggered = false);
-    bool modify(EAlteration, int verticalDistance, bool userTriggered = false, CursorAlignOnScroll = AlignCursorOnScrollIfNeeded);
+    enum VerticalDirection { DirectionUp, DirectionDown };
+    bool modify(EAlteration, unsigned verticalDistance, VerticalDirection, bool userTriggered = false, CursorAlignOnScroll = AlignCursorOnScrollIfNeeded);
     TextGranularity granularity() const { return m_granularity; }