LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 May 2007 03:51:40 +0000 (03:51 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 May 2007 03:51:40 +0000 (03:51 +0000)
        Reviewed by ggaren

        <rdar://problem/5195166> Incorrect proposedRange DOMRange passed to WebViewEditing delegate

        * editing/selection/5195166-1-expected.checksum: Added.
        * editing/selection/5195166-1-expected.png: Added.
        * editing/selection/5195166-1-expected.txt: Added.
        * editing/selection/5195166-1.html: Added.
        * editing/selection/5195166-2-expected.checksum: Added.
        * editing/selection/5195166-2-expected.png: Added.
        * editing/selection/5195166-2-expected.txt: Added.
        * editing/selection/5195166-2.html: Added.

WebCore:

        Reviewed by ggaren

        <rdar://problem/5195166> Incorrect proposedRange DOMRange passed to WebViewEditing delegate

        In setModifyBias, we must cache the start and the end
        because the calls to setBase and setExtent can modify
        them (added a testcase).
        The temporary SelectionController that we use in modify() to
        produce the proposed range that will be passed to
        shouldChangeSelectedDOMRange must have the same m_modifyBias
        as the original SelectionController, or else when the
        modification is performed, setModifyBias can swap the base
        and the extent incorrectly (added a testcase).
        Renamed m_modifyBias to m_lastChangeWasHorizontalExtension.
        Renamed setModifyBias to willBeModified.

        * editing/SelectionController.cpp:
        (WebCore::SelectionController::SelectionController):
        (WebCore::SelectionController::setSelection):
        (WebCore::SelectionController::willBeModified):
        (WebCore::SelectionController::modify):
        * editing/SelectionController.h:
        (WebCore::SelectionController::setLastChangeWasHorizontalExtension):
        * page/EventHandler.cpp:
        (WebCore::EventHandler::handleMousePressEventSingleClick):
        (WebCore::EventHandler::updateSelectionForMouseDragOverPosition):

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/selection/5195166-1-expected.checksum [new file with mode: 0644]
LayoutTests/editing/selection/5195166-1-expected.png [new file with mode: 0644]
LayoutTests/editing/selection/5195166-1-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/5195166-1.html [new file with mode: 0644]
LayoutTests/editing/selection/5195166-2-expected.checksum [new file with mode: 0644]
LayoutTests/editing/selection/5195166-2-expected.png [new file with mode: 0644]
LayoutTests/editing/selection/5195166-2-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/5195166-2.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/SelectionController.cpp
WebCore/editing/SelectionController.h
WebCore/page/EventHandler.cpp

index 0555d06..aca6403 100644 (file)
@@ -1,3 +1,18 @@
+2007-05-10  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by ggaren
+
+        <rdar://problem/5195166> Incorrect proposedRange DOMRange passed to WebViewEditing delegate
+
+        * editing/selection/5195166-1-expected.checksum: Added.
+        * editing/selection/5195166-1-expected.png: Added.
+        * editing/selection/5195166-1-expected.txt: Added.
+        * editing/selection/5195166-1.html: Added.
+        * editing/selection/5195166-2-expected.checksum: Added.
+        * editing/selection/5195166-2-expected.png: Added.
+        * editing/selection/5195166-2-expected.txt: Added.
+        * editing/selection/5195166-2.html: Added.
+
 2007-05-10  Adele Peterson  <adele@apple.com>
 
         Reviewed by Hyatt.
diff --git a/LayoutTests/editing/selection/5195166-1-expected.checksum b/LayoutTests/editing/selection/5195166-1-expected.checksum
new file mode 100644 (file)
index 0000000..088c736
--- /dev/null
@@ -0,0 +1 @@
+8366adb955f518dbe02cbc0650a093ee
\ No newline at end of file
diff --git a/LayoutTests/editing/selection/5195166-1-expected.png b/LayoutTests/editing/selection/5195166-1-expected.png
new file mode 100644 (file)
index 0000000..d5a0920
Binary files /dev/null and b/LayoutTests/editing/selection/5195166-1-expected.png differ
diff --git a/LayoutTests/editing/selection/5195166-1-expected.txt b/LayoutTests/editing/selection/5195166-1-expected.txt
new file mode 100644 (file)
index 0000000..ec59e66
--- /dev/null
@@ -0,0 +1,18 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x576
+      RenderBlock {P} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 704x18
+          text run at (0,0) width 704: "This tests for a bug where extending a selection created with the mouse would blow it away before extending it."
+      RenderBlock {DIV} at (0,34) size 784x18
+        RenderText {#text} at (0,0) size 355x18
+          text run at (0,0) width 355: "There should be five characters selected in this sentence."
+      RenderBlock {UL} at (0,68) size 784x18
+        RenderListItem {LI} at (40,0) size 744x18
+          RenderListMarker at (-17,0) size 7x18: bullet
+          RenderText {#text} at (0,0) size 50x18
+            text run at (0,0) width 50: "Success"
+selection start: position 3 of child 0 {#text} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+selection end:   position 9 of child 0 {#text} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/selection/5195166-1.html b/LayoutTests/editing/selection/5195166-1.html
new file mode 100644 (file)
index 0000000..c78ce2d
--- /dev/null
@@ -0,0 +1,25 @@
+<p>This tests for a bug where extending a selection created with the mouse would blow it away before extending it.</p>
+<div id="div" contenteditable="true">There should be five characters selected in this sentence.</div>
+<ul id="console"></ul>
+<script>
+function log(str) {
+    console = document.getElementById("console");
+    li = document.createElement("li");
+    text = document.createTextNode(str);
+    console.appendChild(li);
+    li.appendChild(text);
+}
+if (window.layoutTestController) {
+    var text = document.getElementById("div").firstChild;
+    var selection = window.getSelection();
+    selection.setBaseAndExtent(text, 3 + 5, text, 3);
+    // Extending this 5 character selection will select 6 characters.
+    textInputController.doCommand("moveForwardAndModifySelection:");
+    // Extending it in this way flips the base and the extent.
+    if (selection.extentOffset - selection.baseOffset != 6)
+        log("Failure: Selection isn't the right size.");
+    else
+        log ("Success");
+} else
+    log ("Failure: This test cannot be run manually.")
+</script>
diff --git a/LayoutTests/editing/selection/5195166-2-expected.checksum b/LayoutTests/editing/selection/5195166-2-expected.checksum
new file mode 100644 (file)
index 0000000..804158b
--- /dev/null
@@ -0,0 +1 @@
+d37efd722d6c73128dc4d9537b9ded16
\ No newline at end of file
diff --git a/LayoutTests/editing/selection/5195166-2-expected.png b/LayoutTests/editing/selection/5195166-2-expected.png
new file mode 100644 (file)
index 0000000..a7b7358
Binary files /dev/null and b/LayoutTests/editing/selection/5195166-2-expected.png differ
diff --git a/LayoutTests/editing/selection/5195166-2-expected.txt b/LayoutTests/editing/selection/5195166-2-expected.txt
new file mode 100644 (file)
index 0000000..485c9a0
--- /dev/null
@@ -0,0 +1,25 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 1 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 1 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 1 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 2 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 2 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 3 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 3 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 2 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x576
+      RenderBlock {P} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 653x18
+          text run at (0,0) width 653: "This tests for a bug where selection change notifications would post the wrong proposed selected range."
+      RenderBlock {DIV} at (0,34) size 784x18
+        RenderText {#text} at (0,0) size 355x18
+          text run at (0,0) width 355: "There should be five characters selected in this sentence."
+      RenderBlock {UL} at (0,68) size 784x0
+selection start: position 0 of child 0 {#text} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+selection end:   position 2 of child 0 {#text} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/selection/5195166-2.html b/LayoutTests/editing/selection/5195166-2.html
new file mode 100644 (file)
index 0000000..a8eb6ae
--- /dev/null
@@ -0,0 +1,25 @@
+<p>This tests for a bug where selection change notifications would post the wrong proposed selected range.</p>
+<div id="div" contenteditable="true">There should be five characters selected in this sentence.</div>
+<ul id="console"></ul>
+<script>
+function log(str) {
+    console = document.getElementById("console");
+    li = document.createElement("li");
+    text = document.createTextNode(str);
+    console.appendChild(li);
+    li.appendChild(text);
+}
+if (window.layoutTestController) {
+    window.layoutTestController.dumpEditingCallbacks();
+    var text = document.getElementById("div").firstChild;
+    var selection = window.getSelection();
+    selection.setBaseAndExtent(text, 0, text, 0);
+    textInputController.doCommand("moveForwardAndModifySelection:");
+    textInputController.doCommand("moveForwardAndModifySelection:");
+    textInputController.doCommand("moveForwardAndModifySelection:");
+    textInputController.doCommand("moveBackwardAndModifySelection:");
+} else {
+    log("Failure: This test cannot be run manually.")
+}
+
+</script>
index 1ea40ff..44dc812 100644 (file)
@@ -1,3 +1,32 @@
+2007-05-10  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by ggaren
+
+        <rdar://problem/5195166> Incorrect proposedRange DOMRange passed to WebViewEditing delegate
+        
+        In setModifyBias, we must cache the start and the end 
+        because the calls to setBase and setExtent can modify 
+        them (added a testcase).
+        The temporary SelectionController that we use in modify() to
+        produce the proposed range that will be passed to 
+        shouldChangeSelectedDOMRange must have the same m_modifyBias
+        as the original SelectionController, or else when the 
+        modification is performed, setModifyBias can swap the base 
+        and the extent incorrectly (added a testcase).
+        Renamed m_modifyBias to m_lastChangeWasHorizontalExtension.
+        Renamed setModifyBias to willBeModified.
+
+        * editing/SelectionController.cpp:
+        (WebCore::SelectionController::SelectionController):
+        (WebCore::SelectionController::setSelection):
+        (WebCore::SelectionController::willBeModified):
+        (WebCore::SelectionController::modify):
+        * editing/SelectionController.h:
+        (WebCore::SelectionController::setLastChangeWasHorizontalExtension):
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::handleMousePressEventSingleClick):
+        (WebCore::EventHandler::updateSelectionForMouseDragOverPosition):
+
 2007-05-10  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Darin Adler.
index 9bad13f..3ce3424 100644 (file)
@@ -59,7 +59,7 @@ const int NoXPosForVerticalArrowNavigation = INT_MIN;
 
 SelectionController::SelectionController(Frame* frame, bool isDragCaretController)
     : m_needsLayout(true)
-    , m_modifyBiasSet(false)
+    , m_lastChangeWasHorizontalExtension(false)
     , m_frame(frame)
     , m_isDragCaretController(isDragCaretController)
     , m_isCaretBlinkingSuspended(false)
@@ -202,26 +202,28 @@ void SelectionController::nodeWillBeRemoved(Node *node)
         setSelection(Selection(), false, false);
 }
 
-void SelectionController::setModifyBias(EAlteration alter, EDirection direction)
+void SelectionController::willBeModified(EAlteration alter, EDirection direction)
 {
     switch (alter) {
         case MOVE:
-            m_modifyBiasSet = false;
+            m_lastChangeWasHorizontalExtension = false;
             break;
         case EXTEND:
-            if (!m_modifyBiasSet) {
-                m_modifyBiasSet = true;
+            if (!m_lastChangeWasHorizontalExtension) {
+                m_lastChangeWasHorizontalExtension = true;
+                Position start = m_sel.start();
+                Position end = m_sel.end();
                 switch (direction) {
                     // FIXME: right for bidi?
                     case RIGHT:
                     case FORWARD:
-                        m_sel.setBase(m_sel.start());
-                        m_sel.setExtent(m_sel.end());
+                        m_sel.setBase(start);
+                        m_sel.setExtent(end);
                         break;
                     case LEFT:
                     case BACKWARD:
-                        m_sel.setBase(m_sel.end());
-                        m_sel.setExtent(m_sel.start());
+                        m_sel.setBase(end);
+                        m_sel.setExtent(start);
                         break;
                 }
             }
@@ -457,6 +459,7 @@ bool SelectionController::modify(EAlteration alter, EDirection dir, TextGranular
 {
     if (userTriggered) {
         SelectionController trialSelectionController;
+        trialSelectionController.setLastChangeWasHorizontalExtension(m_lastChangeWasHorizontalExtension);
         trialSelectionController.setSelection(m_sel);
         trialSelectionController.modify(alter, dir, granularity, false);
 
@@ -468,7 +471,7 @@ bool SelectionController::modify(EAlteration alter, EDirection dir, TextGranular
     if (m_frame)
         m_frame->setSelectionGranularity(granularity);
     
-    setModifyBias(alter, dir);
+    willBeModified(alter, dir);
 
     VisiblePosition pos;
     switch (dir) {
@@ -560,7 +563,7 @@ bool SelectionController::modify(EAlteration alter, int verticalDistance, bool u
     if (up)
         verticalDistance = -verticalDistance;
 
-    setModifyBias(alter, up ? BACKWARD : FORWARD);
+    willBeModified(alter, up ? BACKWARD : FORWARD);
 
     VisiblePosition pos;
     int xPos = 0;
index b2c9c47..8357d99 100644 (file)
@@ -87,8 +87,8 @@ public:
     IntRect caretRect() const;
     void setNeedsLayout(bool flag = true);
 
-    void clearModifyBias() { m_modifyBiasSet = false; }
-    void setModifyBias(EAlteration, EDirection);
+    void setLastChangeWasHorizontalExtension(bool b) { m_lastChangeWasHorizontalExtension = b; }
+    void willBeModified(EAlteration, EDirection);
     
     bool isNone() const { return m_sel.isNone(); }
     bool isCaret() const { return m_sel.isCaret(); }
@@ -184,8 +184,7 @@ private:
     IntPoint m_caretPositionOnLayout;
     
     bool m_needsLayout : 1;       // true if the caret and expectedVisible rectangles need to be calculated
-    bool m_modifyBiasSet : 1;     // true if the selection has been horizontally 
-                                  // modified with EAlteration::EXTEND
+    bool m_lastChangeWasHorizontalExtension : 1;
     Frame* m_frame;
     bool m_isDragCaretController;
 
index 2ed4e2e..7a42e2b 100644 (file)
@@ -234,7 +234,7 @@ bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestR
     
     Selection newSelection = m_frame->selectionController()->selection();
     if (extendSelection && newSelection.isCaretOrRange()) {
-        m_frame->selectionController()->clearModifyBias();
+        m_frame->selectionController()->setLastChangeWasHorizontalExtension(false);
         
         // See <rdar://problem/3668157> REGRESSION (Mail): shift-click deselects when selection 
         // was created right-to-left
@@ -388,7 +388,7 @@ void EventHandler::updateSelectionForMouseDragOverPosition(const VisiblePosition
     // Restart the selection if this is the first mouse move. This work is usually
     // done in handleMousePressEvent, but not if the mouse press was on an existing selection.
     Selection newSelection = m_frame->selectionController()->selection();
-    m_frame->selectionController()->clearModifyBias();
+    m_frame->selectionController()->setLastChangeWasHorizontalExtension(false);
     
     if (!m_beganSelectingText) {
         m_beganSelectingText = true;