LayoutTests:

        Reviewed by Kimon Tsinteris.

        <rdar://problem/5241148> REGRESSION: ActivEdit can't perform operations on certain selections

        Demonstrates bug:
        * editing/selection/5241148-expected.txt: Added.
        * editing/selection/5241148.html: Added.
        Updated to reflect new equivalents for positions before/after tables:
        * editing/selection/click-before-and-after-table-expected.txt:
        * editing/selection/click-before-and-after-table.html:

WebCore:

        Reviewed by Kimon Tsinteris.

        <rdar://problem/5241148> REGRESSION: ActivEdit can't perform operations on certain selections

        Return valid equivalents from these methods (no [img, 1] for
        example). ActivEdit was using positions returned from these
        methods to create new DOM Ranges.

        * editing/SelectionController.cpp:
        (WebCore::SelectionController::baseNode):
        (WebCore::SelectionController::baseOffset):
        (WebCore::SelectionController::extentNode):
        (WebCore::SelectionController::extentOffset):
        (WebCore::SelectionController::anchorNode):
        (WebCore::SelectionController::anchorOffset):
        (WebCore::SelectionController::focusNode):
        (WebCore::SelectionController::focusOffset):
        * editing/SelectionController.h:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 5724e25fba65b8fe4f64c668832fcebc42c07752..079d6f6abf7067947b87b845cba9a74d8278ae04 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2007-06-04  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Kimon Tsinteris.
+        
+        <rdar://problem/5241148> REGRESSION: ActivEdit can't perform operations on certain selections
+
+        Demonstrates bug:
+        * editing/selection/5241148-expected.txt: Added.
+        * editing/selection/5241148.html: Added.
+        Updated to reflect new equivalents for positions before/after tables:
+        * editing/selection/click-before-and-after-table-expected.txt:
+        * editing/selection/click-before-and-after-table.html:
+
 2007-06-04  Rob Buis  <buis@kde.org>
 
         Reviewed by Hyatt.

diff --git a/LayoutTests/editing/selection/5241148-expected.txt b/LayoutTests/editing/selection/5241148-expected.txt
new file mode 100644 (file)
index 0000000..f31f41d
--- /dev/null
+++ b/LayoutTests/editing/selection/5241148-expected.txt
@@ -0,0 +1,5 @@
+ALERT: Success
+This tests to see that the Selection object's properties return valid DOM positions (not our internal editing positions). We should be able to create Range objects out of those positions w/o issue.
+
+foo
+

diff --git a/LayoutTests/editing/selection/5241148.html b/LayoutTests/editing/selection/5241148.html
new file mode 100644 (file)
index 0000000..4a07cad
--- /dev/null
+++ b/LayoutTests/editing/selection/5241148.html
@@ -0,0 +1,24 @@
+<p>This tests to see that the Selection object's properties return valid DOM positions (not our internal editing positions). We should be able to create Range objects out of those positions w/o issue.</p>
+<div id="div" contenteditable="true">foo<img src="../resources/abe.png"><br></div>
+
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpAsText();
+
+div = document.getElementById("div");
+selection = window.getSelection();
+try {
+    selection.setBaseAndExtent(div, 0, div, 2);
+    extentNode = selection.extentNode;
+    extentOffset = selection.extentOffset;
+    range = document.createRange();
+    range.setStart(extentNode, extentOffset);
+    range.setEnd(extentNode, extentOffset);
+    selection.removeAllRanges();
+    selection.addRange(range);
+    alert("Success");
+
+} catch (e) {
+    alert("Failed: " + e);
+}
+</script>

diff --git a/LayoutTests/editing/selection/click-before-and-after-table-expected.txt b/LayoutTests/editing/selection/click-before-and-after-table-expected.txt
index ca081f9a755e44cb78ba3d47b27fb4ce1be464f5..13b48d2e65b55b20cc753157b6fec20bff5f18e4 100644 (file)
--- a/LayoutTests/editing/selection/click-before-and-after-table-expected.txt
+++ b/LayoutTests/editing/selection/click-before-and-after-table-expected.txt
@@ -1,12 +1,12 @@
-EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of BODY > HTML > #document to 6 of BODY > HTML > #document
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of BODY > HTML > #document to 5 of BODY > HTML > #document
 EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 2 of BODY > HTML > #document to 2 of BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of BODY > HTML > #document to 1 of BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 2 of BODY > HTML > #document to 2 of BODY > HTML > #document toDOMRange:range from 8 of #text > TD > TR > TBODY > TABLE > BODY > HTML > #document to 8 of #text > TD > TR > TBODY > TABLE > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of BODY > HTML > #document to 1 of BODY > HTML > #document toDOMRange:range from 8 of #text > TD > TR > TBODY > TABLE > BODY > HTML > #document to 8 of #text > TD > TR > TBODY > TABLE > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 8 of #text > TD > TR > TBODY > TABLE > BODY > HTML > #document to 8 of #text > TD > TR > TBODY > TABLE > BODY > HTML > #document toDOMRange:range from 1 of BODY > HTML > #document to 1 of BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 8 of #text > TD > TR > TBODY > TABLE > BODY > HTML > #document to 8 of #text > TD > TR > TBODY > TABLE > BODY > HTML > #document toDOMRange:range from 0 of BODY > HTML > #document to 0 of BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of BODY > HTML > #document to 1 of BODY > HTML > #document toDOMRange:range from 0 of TD > TR > TBODY > TABLE > BODY > HTML > #document to 0 of TD > TR > TBODY > TABLE > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of BODY > HTML > #document to 0 of BODY > HTML > #document toDOMRange:range from 0 of TD > TR > TBODY > TABLE > BODY > HTML > #document to 0 of TD > TR > TBODY > TABLE > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 cell one       cell two
 

diff --git a/LayoutTests/editing/selection/click-before-and-after-table.html b/LayoutTests/editing/selection/click-before-and-after-table.html
index 921fe9e387d2e218246a79c8076eb7e5ee6a3ff1..3c059b7c03f39bb6c786d41e0c461667973cf480 100644 (file)
--- a/LayoutTests/editing/selection/click-before-and-after-table.html
+++ b/LayoutTests/editing/selection/click-before-and-after-table.html
@@ -8,10 +8,7 @@ td {
 }
 </style>
 
-<body style="border: 1px solid red;" contenteditable="true">
-<table id="table" style="margin: 25px; border:10px solid #ccc; padding: 10px;">
-<tr><td>cell one</td><td>cell two</td></tr>
-</table>
+<body style="border: 1px solid red;" contenteditable="true"><table id="table" style="margin: 25px; border:10px solid #ccc; padding: 10px;"><tr><td>cell one</td><td>cell two</td></tr></table>
 <ul id="console"></ul>
 <script>
 function log(message) {
@@ -40,7 +37,7 @@ else {
     eventSender.mouseDown();
     eventSender.mouseUp();
     s = window.getSelection();
-    if (s.anchorNode != table || s.anchorOffset != table.childNodes.length)
+    if (!(s.anchorNode == document.body && s.anchorOffset == 1))
         log("Failure: Clicking after the table didn't put the caret after it.");
     
     x = right - 5;
@@ -50,7 +47,7 @@ else {
     eventSender.mouseDown();
     eventSender.mouseUp();
     s = window.getSelection();
-    if (s.anchorNode == table)
+    if (s.anchorNode == document.body)
         log("Failure: Clicking inside the table put the caret before or after it.");
         
     x = left - 5;
@@ -61,6 +58,7 @@ else {
     eventSender.mouseUp();
     s = window.getSelection();
     if (s.anchorNode != table || s.anchorOffset != 0)
+    if (!(s.anchorNode == document.body && s.anchorOffset == 0))
         log("Failure: Clicking before the table should be the caret before it.");
         
     x = left + 5;
@@ -70,7 +68,7 @@ else {
     eventSender.mouseDown();
     eventSender.mouseUp();
     s = window.getSelection();
-    if (s.anchorNode == table)
+    if (s.anchorNode == document.body)
         log("Failure: Clicking inside the table put the caret before or after it.");
 }
 </script>

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 1a73ff52ddeedfa139f23e69e614a2641b9c463d..229cbbee48d560449529ba78d9488dcc5e8e5f9e 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2007-06-04  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Kimon Tsinteris.
+
+        <rdar://problem/5241148> REGRESSION: ActivEdit can't perform operations on certain selections
+        
+        Return valid equivalents from these methods (no [img, 1] for 
+        example). ActivEdit was using positions returned from these
+        methods to create new DOM Ranges.
+        
+        * editing/SelectionController.cpp:
+        (WebCore::SelectionController::baseNode):
+        (WebCore::SelectionController::baseOffset):
+        (WebCore::SelectionController::extentNode):
+        (WebCore::SelectionController::extentOffset):
+        (WebCore::SelectionController::anchorNode):
+        (WebCore::SelectionController::anchorOffset):
+        (WebCore::SelectionController::focusNode):
+        (WebCore::SelectionController::focusOffset):
+        * editing/SelectionController.h:
+
 2007-06-04  Rob Buis  <buis@kde.org>
 
         Fix the svg experimental build.

diff --git a/WebCore/editing/SelectionController.cpp b/WebCore/editing/SelectionController.cpp
index cab644042fce0b523e629e2e440ed847a70e84a3..e7db293d0019dd9ca792a61425e10a94ad376ee2 100644 (file)
--- a/WebCore/editing/SelectionController.cpp
+++ b/WebCore/editing/SelectionController.cpp
@@ -715,6 +715,59 @@ String SelectionController::type() const
         return "Range";
 }
 
+// These methods are accessible via JS (and are not used internally), so they must return valid DOM positions.
+Node* SelectionController::baseNode() const
+{
+    Position base = rangeCompliantEquivalent(m_sel.base());
+    return base.node();
+}
+
+int SelectionController::baseOffset() const
+{
+    Position base = rangeCompliantEquivalent(m_sel.base());
+    return base.offset();
+}
+
+Node* SelectionController::extentNode() const
+{
+    Position extent = rangeCompliantEquivalent(m_sel.extent());
+    return extent.node();
+}
+
+int SelectionController::extentOffset() const
+{
+    Position extent = rangeCompliantEquivalent(m_sel.extent());
+    return extent.offset();
+}
+
+Node* SelectionController::anchorNode() const
+{
+    Position anchor = m_sel.isBaseFirst() ? m_sel.start() : m_sel.end();
+    anchor = rangeCompliantEquivalent(anchor);
+    return anchor.node();
+}
+
+int SelectionController::anchorOffset() const
+{
+    Position anchor = m_sel.isBaseFirst() ? m_sel.start() : m_sel.end();
+    anchor = rangeCompliantEquivalent(anchor);
+    return anchor.offset();
+}
+
+Node* SelectionController::focusNode() const
+{
+    Position focus = m_sel.isBaseFirst() ? m_sel.end() : m_sel.start();
+    focus = rangeCompliantEquivalent(focus);
+    return focus.node();
+}
+
+int SelectionController::focusOffset() const
+{
+    Position focus = m_sel.isBaseFirst() ? m_sel.end() : m_sel.start();
+    focus = rangeCompliantEquivalent(focus);
+    return focus.offset();
+}
+
 String SelectionController::toString() const
 {
     return String(plainText(m_sel.toRange().get()));

diff --git a/WebCore/editing/SelectionController.h b/WebCore/editing/SelectionController.h
index 05b9deac2f5a777de3a899f194a3c43e924a5c6c..bd96c61d8a3595e24ff43144a3b88edae7dd9501 100644 (file)
--- a/WebCore/editing/SelectionController.h
+++ b/WebCore/editing/SelectionController.h
@@ -104,10 +104,11 @@ public:
     void nodeWillBeRemoved(Node*);
 
     // Safari Selection Object API
-    Node* baseNode() const { return m_sel.base().node(); }
-    Node* extentNode() const { return m_sel.extent().node(); }
-    int baseOffset() const { return m_sel.base().offset(); }
-    int extentOffset() const { return m_sel.extent().offset(); }
+    // These methods return the valid equivalents of internal editing positions.
+    Node* baseNode() const;
+    Node* extentNode() const;
+    int baseOffset() const;
+    int extentOffset() const;
     String type() const;
     void setBaseAndExtent(Node* baseNode, int baseOffset, Node* extentNode, int extentOffset, ExceptionCode&);
     void setPosition(Node*, int offset, ExceptionCode&);
@@ -118,10 +119,11 @@ public:
     // but reflect the direction in which the selection was made by the user.  That does
     // not mean that they are base/extent, since the base/extent don't reflect
     // expansion.
-    Node* anchorNode() const { return m_sel.isBaseFirst() ? m_sel.start().node() : m_sel.end().node(); }
-    int anchorOffset() const { return m_sel.isBaseFirst() ? m_sel.start().offset() : m_sel.end().offset(); }
-    Node* focusNode() const { return m_sel.isBaseFirst() ? m_sel.end().node() : m_sel.start().node(); }
-    int focusOffset() const { return m_sel.isBaseFirst() ? m_sel.end().offset() : m_sel.start().offset(); }
+    // These methods return the valid equivalents of internal editing positions.
+    Node* anchorNode() const;
+    int anchorOffset() const;
+    Node* focusNode() const;
+    int focusOffset() const;
     bool isCollapsed() const { return !isRange(); }
     String toString() const;
     void collapse(Node*, int offset, ExceptionCode&);