LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 May 2006 02:23:27 +0000 (02:23 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 May 2006 02:23:27 +0000 (02:23 +0000)
        Reviewed by levi

        Added, test for the crash by focusing an editable html element and then
        inserting a tab
        * editing/selection/editable-html-element-expected.checksum: Added.
        * editing/selection/editable-html-element-expected.png: Added.
        * editing/selection/editable-html-element-expected.txt: Added.
        * editing/selection/editable-html-element.html: Added.
        Fixed:
        * editing/selection/focus_editable_html-expected.checksum:
        * editing/selection/focus_editable_html-expected.png:
        * editing/selection/focus_editable_html-expected.txt:

WebCore:

        Reviewed by levi

        <rdar://problem/4564296> Mail crashes on Leopard9A184 when I attempt to compose a new message

        * dom/Position.cpp:
        (WebCore::Position::inRenderedContent):
        Removed a candidate at [html, 0].
        * editing/CreateLinkCommand.cpp:
        (WebCore::CreateLinkCommand::doApply): Added early return when there is no selection.
        * editing/InsertLineBreakCommand.cpp:
        (WebCore::InsertLineBreakCommand::doApply): Ditto.
        * editing/InsertTextCommand.cpp:
        (WebCore::InsertTextCommand::input): Ditto.
        * editing/Selection.cpp:
        (WebCore::Selection::validate): If visible positions can't be created from the endpoints,
        then create a null selection.  Not doing this was making editing code think there was
        a valid, editable selection even though there wasn't.
        * editing/UnlinkCommand.cpp:
        (WebCore::UnlinkCommand::doApply): Early return.
        * editing/VisiblePosition.cpp:
        (WebCore::VisiblePosition::initDeepPosition): Special case the html/body element boundary.
        It looks like a non-editable/editable boundary since rootEditableElement stops at the body
        even if the html element is editable.

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/selection/editable-html-element-expected.checksum [new file with mode: 0644]
LayoutTests/editing/selection/editable-html-element-expected.png [new file with mode: 0644]
LayoutTests/editing/selection/editable-html-element-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/editable-html-element.html [new file with mode: 0644]
LayoutTests/editing/selection/focus_editable_html-expected.checksum
LayoutTests/editing/selection/focus_editable_html-expected.png
LayoutTests/editing/selection/focus_editable_html-expected.txt
WebCore/ChangeLog
WebCore/dom/Position.cpp
WebCore/editing/CreateLinkCommand.cpp
WebCore/editing/InsertLineBreakCommand.cpp
WebCore/editing/InsertTextCommand.cpp
WebCore/editing/Selection.cpp
WebCore/editing/UnlinkCommand.cpp
WebCore/editing/VisiblePosition.cpp

index 50124b0947a004e7787b0b35bae4e857176918b1..79927feb6a8cf4c59b72bf68c23b2faa53bfa89e 100644 (file)
@@ -1,3 +1,18 @@
+2006-05-26  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by levi
+
+        Added, test for the crash by focusing an editable html element and then
+        inserting a tab
+        * editing/selection/editable-html-element-expected.checksum: Added.
+        * editing/selection/editable-html-element-expected.png: Added.
+        * editing/selection/editable-html-element-expected.txt: Added.
+        * editing/selection/editable-html-element.html: Added.
+        Fixed:
+        * editing/selection/focus_editable_html-expected.checksum:
+        * editing/selection/focus_editable_html-expected.png:
+        * editing/selection/focus_editable_html-expected.txt:
+
 2006-05-26  Adele Peterson  <adele@apple.com>
 
         Reviewed by Hyatt.
diff --git a/LayoutTests/editing/selection/editable-html-element-expected.checksum b/LayoutTests/editing/selection/editable-html-element-expected.checksum
new file mode 100644 (file)
index 0000000..e3611b0
--- /dev/null
@@ -0,0 +1 @@
+934e535284ce78b20955e9f09865cd7e
\ No newline at end of file
diff --git a/LayoutTests/editing/selection/editable-html-element-expected.png b/LayoutTests/editing/selection/editable-html-element-expected.png
new file mode 100644 (file)
index 0000000..601b3b7
Binary files /dev/null and b/LayoutTests/editing/selection/editable-html-element-expected.png differ
diff --git a/LayoutTests/editing/selection/editable-html-element-expected.txt b/LayoutTests/editing/selection/editable-html-element-expected.txt
new file mode 100644 (file)
index 0000000..64ae771
--- /dev/null
@@ -0,0 +1,29 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of HTML > #document to 2 of HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) 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 0 of BODY > HTML > #document to 0 of BODY > HTML > #document toDOMRange:range from 1 of #text > SPAN > BODY > HTML > #document to 1 of #text > SPAN > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > SPAN > BODY > HTML > #document to 1 of #text > SPAN > BODY > HTML > #document toDOMRange:range from 77 of #text > BODY > HTML > #document to 77 of #text > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 77 of #text > BODY > HTML > #document to 77 of #text > BODY > HTML > #document toDOMRange:range from 153 of #text > BODY > HTML > #document to 153 of #text > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+layer at (0,0) size 800x600
+  RenderCanvas at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderInline {SPAN} at (0,0) size 32x18
+        RenderText {#text} at (0,0) size 32x18
+          text run at (0,0) width 32: "\x{9}"
+      RenderText {#text} at (32,0) size 473x18
+        text run at (32,0) width 473: "This tests to see where the selection is set when an html element is focused."
+      RenderBR {BR} at (505,14) size 0x0
+      RenderText {#text} at (0,18) size 777x36
+        text run at (0,18) width 520: "We set it inside the body because we don't want to allow editing outside the body. "
+        text run at (520,18) width 257: "This test also does some editing to make "
+        text run at (0,36) width 173: "sure it happens in the body."
+caret: position 153 of child 3 {#text} of child 1 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/selection/editable-html-element.html b/LayoutTests/editing/selection/editable-html-element.html
new file mode 100644 (file)
index 0000000..60ab42b
--- /dev/null
@@ -0,0 +1,15 @@
+<html contenteditable="true" id="test">
+<script>
+function runTest() {
+    var s = window.getSelection();
+    var e = document.getElementById("test");
+
+    e.focus();
+    document.execCommand("InsertText", false, "\t");
+    document.execCommand("InsertText", false, "This tests to see where the selection is set when an html element is focused.");
+    document.execCommand("InsertHTML", false, "<br>We set it inside the body because we don't want to allow editing outside the body.  This test also does some editing to make sure it happens in the body.");
+}
+</script>
+<body onLoad="runTest();">
+</body>
+</html>
\ No newline at end of file
index f1f661179bae324f978c5da23d2f8d6120f61840..e5061dea488a58f1731cf1bc77f9e64296a783cc 100644 (file)
@@ -1 +1 @@
-2b32e27839b60eda65fb9513e21e39be
\ No newline at end of file
+9e5e54bf429185536353247bf42831c1
\ No newline at end of file
index b64607220ea9e97c24a645719b06ac17bea9e6cf..fea3c688b3870144ea30504b60e4b7d586470889 100644 (file)
Binary files a/LayoutTests/editing/selection/focus_editable_html-expected.png and b/LayoutTests/editing/selection/focus_editable_html-expected.png differ
index cbe1da4e8a2c065cdd5bd726307bef74d8e25dc5..6ed0eb056ebfacd827a803060278bb0999dfda30 100644 (file)
@@ -1,6 +1,6 @@
 EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of HTML > #document to 1 of HTML > #document
 EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of HTML > #document to 0 of HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > BODY > HTML > #document to 42 of #text > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 layer at (0,0) size 2008x2088
   RenderCanvas at (0,0) size 785x585
@@ -23,4 +23,6 @@ layer at (0,0) size 2008x2088
         RenderText {#text} at (0,0) size 237x18
           text run at (0,0) width 237: "If the document scrolls here, test fails."
         RenderText {#text} at (0,0) size 0x0
-caret: position 0 of child 0 {HTML} of document
+selection start: position 1 of child 0 {#text} of child 0 {BODY} of child 0 {HTML} of document
+selection end:   position 42 of child 8 {#text} of child 0 {BODY} of child 0 {HTML} of document
+scrolled to 0,8
index 606166dc5d7088bf89b85bbfdab582bbc617426e..b185374f9b5227165c81a91957fdde04fa925091 100644 (file)
@@ -1,3 +1,29 @@
+2006-05-26  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by levi
+        
+        <rdar://problem/4564296> Mail crashes on Leopard9A184 when I attempt to compose a new message
+
+        * dom/Position.cpp:
+        (WebCore::Position::inRenderedContent):
+        Removed a candidate at [html, 0].
+        * editing/CreateLinkCommand.cpp:
+        (WebCore::CreateLinkCommand::doApply): Added early return when there is no selection.
+        * editing/InsertLineBreakCommand.cpp:
+        (WebCore::InsertLineBreakCommand::doApply): Ditto.
+        * editing/InsertTextCommand.cpp:
+        (WebCore::InsertTextCommand::input): Ditto.
+        * editing/Selection.cpp:
+        (WebCore::Selection::validate): If visible positions can't be created from the endpoints, 
+        then create a null selection.  Not doing this was making editing code think there was
+        a valid, editable selection even though there wasn't.
+        * editing/UnlinkCommand.cpp:
+        (WebCore::UnlinkCommand::doApply): Early return.
+        * editing/VisiblePosition.cpp:
+        (WebCore::VisiblePosition::initDeepPosition): Special case the html/body element boundary.
+        It looks like a non-editable/editable boundary since rootEditableElement stops at the body
+        even if the html element is editable.
+
 2006-05-26  Adele Peterson  <adele@apple.com>
 
         Reviewed by Justin.
index 2ba2210e61683257d302ed80d2efa29a3d45f9e0..83be2a3804dc742d344d4275bc37e07e158a1645 100644 (file)
@@ -478,7 +478,7 @@ bool Position::inRenderedContent() const
     if (isTableElement(node()) || editingIgnoresContent(node()))
         return offset() == 0 || offset() == maxDeepOffset(node());
 
-    if (renderer->isBlockFlow() && !hasRenderedChildrenWithHeight(renderer) &&
+    if (!node()->hasTagName(htmlTag) && renderer->isBlockFlow() && !hasRenderedChildrenWithHeight(renderer) &&
        (renderer->height() || node()->hasTagName(bodyTag)))
         return offset() == 0;
     
index dbbe0c4b5462cc256d42d6e1be6188bf7632db95..9a5651a5bb4651fd81c33d0e4438b060c27ac40a 100644 (file)
@@ -38,6 +38,9 @@ CreateLinkCommand::CreateLinkCommand(Document* document, const String& url)
 
 void CreateLinkCommand::doApply()
 {
+    if (!endingSelection().isRange())
+        return;
+        
     pushPartiallySelectedAnchorElementsDown();
 
     HTMLAnchorElement* anchorElement = new HTMLAnchorElement(document());
index 7b4ca3c41e9874267c3249000a102677329f921a..361ce9af23e273624540a2255e4e3fa99cad101d 100644 (file)
@@ -82,6 +82,8 @@ void InsertLineBreakCommand::doApply()
 {
     deleteSelection();
     Selection selection = endingSelection();
+    if (selection.isNone())
+        return;
 
     RefPtr<Element> breakNode = createBreakElement(document());
     Node* nodeToInsert = breakNode.get();
index e25904d9520cddd1d2afb9e9fce8939f47a7a650..4611e45048fdfa228fa0f44a537ffd493f4e27a5 100644 (file)
@@ -86,6 +86,9 @@ void InsertTextCommand::input(const String &text, bool selectInsertedText)
     assert(text.find('\n') == -1);
 
     Selection selection = endingSelection();
+    if (endingSelection().isNone())
+        return;
+    
     // FIXME: I don't see how "start of line" could possibly be the right check here.
     // It depends on line breaks which depends on the way things are current laid out,
     // window width, etc. This needs to be rethought.
@@ -149,6 +152,7 @@ void InsertTextCommand::input(const String &text, bool selectInsertedText)
 WebCore::Position InsertTextCommand::insertTab(Position pos)
 {
     Position insertPos = VisiblePosition(pos, DOWNSTREAM).deepEquivalent();
+        
     Node *node = insertPos.node();
     unsigned int offset = insertPos.offset();
 
index bba4149936936cc76e83dc159227f4250d291979..9a191b93355f539643c2a2aa2ddffae18c6b2e44 100644 (file)
@@ -173,29 +173,19 @@ static int comparePositions(const Position& a, const Position& b)
 void Selection::validate()
 {
     // Move the selection to rendered positions, if possible.
-    Position originalBase(m_base);
     bool baseAndExtentEqual = m_base == m_extent;
     if (m_base.isNotNull()) {
         m_base = VisiblePosition(m_base, m_affinity).deepEquivalent();
         if (baseAndExtentEqual)
             m_extent = m_base;
     }
-    if (m_extent.isNotNull() && !baseAndExtentEqual) {
+    if (m_extent.isNotNull() && !baseAndExtentEqual)
         m_extent = VisiblePosition(m_extent, m_affinity).deepEquivalent();
-    }
 
     // Make sure we do not have a dangling start or end
-    if (m_base.isNull() && m_extent.isNull()) {
-        // Move the position to the enclosingBlockFlowElement of the original base, if possible.
-        // This has the effect of flashing the caret somewhere when a rendered position for
-        // the base and extent cannot be found.
-        if (originalBase.isNotNull()) {
-            Position pos(originalBase.node()->enclosingBlockFlowElement(), 0);
-            m_base = pos;
-            m_extent = pos;
-        }
+    if (m_base.isNull() && m_extent.isNull())
         m_baseIsFirst = true;
-    else if (m_base.isNull()) {
+    else if (m_base.isNull()) {
         m_base = m_extent;
         m_baseIsFirst = true;
     } else if (m_extent.isNull()) {
index 32dbc856076c8070eb67fa9af7dd716ed9a1e8c8..0ba9a06bc5a46b8e39178fb60cfe8b83eefcf08b 100644 (file)
@@ -37,6 +37,10 @@ UnlinkCommand::UnlinkCommand(Document* document)
 
 void UnlinkCommand::doApply()
 {
+    // FIXME: If a caret is inside a link, remove it.
+    if (!endingSelection().isRange())
+        return;
+        
     pushPartiallySelectedAnchorElementsDown();
 
     HTMLAnchorElement* anchorElement = new HTMLAnchorElement(document());
index e1c831e2ef829a886a4b6481489d3b6398ecaf35..cd6ad595d3606af53878fddb038dc8532cb937f3 100644 (file)
@@ -168,6 +168,13 @@ void VisiblePosition::initDeepPosition(const Position& position, EAffinity affin
 
     // The new position must be in the same editable element. Enforce that first.
     Node* editingRoot = node->rootEditableElement();
+    // If the html element is editable, descending into its body will look like a descent 
+    // from non-editable to editable content since rootEditableElement stops at the body 
+    // even if the html element is editable.
+    if (editingRoot && editingRoot->hasTagName(htmlTag)) {
+        m_deepPosition = next.isNotNull() ? next : prev;
+        return;
+    }
     bool prevIsInSameEditableElement = prevNode && prevNode->rootEditableElement() == editingRoot;
     bool nextIsInSameEditableElement = nextNode && nextNode->rootEditableElement() == editingRoot;
     if (prevIsInSameEditableElement && !nextIsInSameEditableElement) {