2011-06-20 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 20 Jun 2011 18:28:48 +0000 (18:28 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 20 Jun 2011 18:28:48 +0000 (18:28 +0000)
        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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@89263 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/editing/EditorCommand.cpp
Source/WebCore/editing/FrameSelection.cpp
Source/WebCore/editing/FrameSelection.h

index 6a086e4..5708ecc 100644 (file)
@@ -1,3 +1,24 @@
+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>
 
         Reviewed by Adam Barth.
index cd80118..e6c789e 100644 (file)
@@ -245,7 +245,7 @@ static TriState stateTextWritingDirection(Frame* frame, WritingDirection directi
     return (selectionDirection == direction && !hasNestedOrMultipleEmbeddings) ? TrueTriState : FalseTriState;
 }
 
-static int verticalScrollDistance(Frame* frame)
+static unsigned verticalScrollDistance(Frame* frame)
 {
     Node* focusedNode = frame->document()->focusedNode();
     if (!focusedNode)
@@ -258,9 +258,8 @@ static int verticalScrollDistance(Frame* frame)
         return 0;
     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));
 }
 
 static RefPtr<Range> unionDOMRanges(Range* a, Range* b)
@@ -653,34 +652,34 @@ static bool executeMoveLeftAndModifySelection(Frame* frame, Event*, EditorComman
 
 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);
 }
 
 static bool executeMoveRight(Frame* frame, Event*, EditorCommandSource, const String&)
index cefda54..451fd7b 100644 (file)
@@ -888,7 +888,7 @@ static bool absoluteCaretY(const VisiblePosition &c, int &y)
     return true;
 }
 
-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)
         return false;
@@ -897,26 +897,22 @@ bool FrameSelection::modify(EAlteration alter, int verticalDistance, bool userTr
         FrameSelection trialFrameSelection;
         trialFrameSelection.setSelection(m_selection);
         trialFrameSelection.setIsDirectional(m_isDirectional);
-        trialFrameSelection.modify(alter, verticalDistance, false);
+        trialFrameSelection.modify(alter, verticalDistance, direction, false);
 
         bool change = shouldChangeSelection(trialFrameSelection.selection());
         if (!change)
             return false;
     }
 
-    bool up = verticalDistance < 0;
-    if (up)
-        verticalDistance = -verticalDistance;
-
-    willBeModified(alter, up ? DirectionBackward : DirectionForward);
+    willBeModified(alter, direction == DirectionUp ? DirectionBackward : DirectionForward);
 
     VisiblePosition pos;
     int xPos = 0;
     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:
         pos = VisiblePosition(m_selection.extent(), m_selection.affinity());
@@ -928,22 +924,22 @@ bool FrameSelection::modify(EAlteration alter, int verticalDistance, bool userTr
     int startY;
     if (!absoluteCaretY(pos, startY))
         return false;
-    if (up)
+    if (direction == DirectionUp)
         startY = -startY;
     int lastY = startY;
 
     VisiblePosition result;
     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;
         int nextY;
         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) {
             lastY = nextY;
index 9a2b1de..6e62852 100644 (file)
@@ -146,7 +146,8 @@ public:
     EAffinity affinity() const { return m_selection.affinity(); }
 
     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; }
 
     void setStart(const VisiblePosition &, bool userTriggered = false);