LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Nov 2006 21:25:28 +0000 (21:25 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Nov 2006 21:25:28 +0000 (21:25 +0000)
        Reviewed by harrison

        <rdar://problem/4397952>
        Cannot select buttons at the end of a document, preventing copy/paste

        * editing/selection/4397952-expected.checksum: Added.
        * editing/selection/4397952-expected.png: Added.
        * editing/selection/4397952-expected.txt: Added.
        * editing/selection/4397952.html: Added.

WebCore:

        Reviewed by harrison

        <rdar://problem/4397952>
        Cannot select buttons at the end of a document, preventing copy/paste

        There were no VisiblePositions before/after buttons because editingIgnoresContent
        returned false for buttons.

        * dom/Position.cpp:
        (WebCore::Position::upstream): Fixed a comment.
        (WebCore::Position::downstream): Ditto.
        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::initializeStartEnd): Ditto.
        * editing/htmlediting.cpp:
        (WebCore::editingIgnoresContent): It's unnecessary to prefer renderer
        checks over tag name checks because it seems that a node of a tag name
        that we do not ignore content for can't have a renderer of a type that we do.
        (WebCore::canHaveChildrenForEditing): Added selects, buttons, applets, and embeds.
        * editing/visible_units.cpp:
        (WebCore::previousLinePosition): Migrate to enclosingBlock.  Fixes a bug where the
        caret would get stuck moving up/down a line from a caret just before an
        input element.
        (WebCore::nextLinePosition): Ditto.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/4397952-expected.checksum [new file with mode: 0644]
LayoutTests/editing/selection/4397952-expected.png [new file with mode: 0644]
LayoutTests/editing/selection/4397952-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/4397952.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/dom/Position.cpp
WebCore/editing/DeleteSelectionCommand.cpp
WebCore/editing/htmlediting.cpp
WebCore/editing/visible_units.cpp

index 7006730a50067e46cf52a9c0c03b5bcce926c038..3f1bf69c2c1f5e1c1f921cfc276d775e01ca840f 100644 (file)
@@ -1,3 +1,15 @@
+2006-11-28  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by harrison
+        
+        <rdar://problem/4397952>
+        Cannot select buttons at the end of a document, preventing copy/paste
+
+        * editing/selection/4397952-expected.checksum: Added.
+        * editing/selection/4397952-expected.png: Added.
+        * editing/selection/4397952-expected.txt: Added.
+        * editing/selection/4397952.html: Added.
+
 2006-11-27  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Hyatt.
diff --git a/LayoutTests/editing/selection/4397952-expected.checksum b/LayoutTests/editing/selection/4397952-expected.checksum
new file mode 100644 (file)
index 0000000..de6e43b
--- /dev/null
@@ -0,0 +1 @@
+ab727d7f430ce69cbf001ec6085be8d7
\ No newline at end of file
diff --git a/LayoutTests/editing/selection/4397952-expected.png b/LayoutTests/editing/selection/4397952-expected.png
new file mode 100644 (file)
index 0000000..cffe80c
Binary files /dev/null and b/LayoutTests/editing/selection/4397952-expected.png differ
diff --git a/LayoutTests/editing/selection/4397952-expected.txt b/LayoutTests/editing/selection/4397952-expected.txt
new file mode 100644 (file)
index 0000000..2e644e1
--- /dev/null
@@ -0,0 +1,25 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 4 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+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 784x584
+      RenderBlock {P} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 565x18
+          text run at (0,0) width 264: "This tests caret movement across buttons. "
+          text run at (264,0) width 301: "The caret should be just after the second button."
+      RenderBlock {DIV} at (0,34) size 784x22
+        RenderButton {INPUT} at (2,2) size 36x18 [bgcolor=#C0C0C0]
+          RenderBlock (anonymous) at (8,2) size 20x13
+            RenderText at (0,0) size 20x13
+              text run at (0,0) width 20: "Foo"
+        RenderButton {INPUT} at (42,2) size 33x18 [bgcolor=#C0C0C0]
+          RenderBlock (anonymous) at (8,2) size 17x13
+            RenderText at (0,0) size 17x13
+              text run at (0,0) width 17: "Bar"
+        RenderText {#text} at (0,0) size 0x0
+caret: position 1 of child 2 {INPUT} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/selection/4397952.html b/LayoutTests/editing/selection/4397952.html
new file mode 100644 (file)
index 0000000..5cff565
--- /dev/null
@@ -0,0 +1,18 @@
+<p>This tests caret movement across buttons.  The caret should be just after the second button.</p>
+<div id="div" contenteditable="true">
+<input type="submit" value="Foo"><input type="reset" value="Bar">
+</div>
+
+<script type="Javascript" src="../editing.js"></script>
+<script>
+runEditingTest();
+
+function editingTest() {
+    var sel = window.getSelection();
+    var div = document.getElementById("div");
+    sel.setPosition(div, 0);
+    
+    moveSelectionForwardByCharacterCommand();
+    moveSelectionForwardByCharacterCommand();
+}
+</script>
\ No newline at end of file
index 5f27a2483583a2f652511a08138728b6f25202a8..665243c4894dd8dd733c03507263ebf7ce75fc20 100644 (file)
@@ -1,3 +1,29 @@
+2006-11-28  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by harrison
+
+        <rdar://problem/4397952>
+        Cannot select buttons at the end of a document, preventing copy/paste
+        
+        There were no VisiblePositions before/after buttons because editingIgnoresContent
+        returned false for buttons. 
+
+        * dom/Position.cpp:
+        (WebCore::Position::upstream): Fixed a comment.
+        (WebCore::Position::downstream): Ditto.
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::initializeStartEnd): Ditto.
+        * editing/htmlediting.cpp:
+        (WebCore::editingIgnoresContent): It's unnecessary to prefer renderer 
+        checks over tag name checks because it seems that a node of a tag name 
+        that we do not ignore content for can't have a renderer of a type that we do.
+        (WebCore::canHaveChildrenForEditing): Added selects, buttons, applets, and embeds.
+        * editing/visible_units.cpp:
+        (WebCore::previousLinePosition): Migrate to enclosingBlock.  Fixes a bug where the
+        caret would get stuck moving up/down a line from a caret just before an 
+        input element.
+        (WebCore::nextLinePosition): Ditto.
+
 2006-11-28  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Adam.
index 03f3a51f126e2002ba67173ac618400c871e6d39..8b23b252e9ca2018744072ec037af25fbec307ed 100644 (file)
@@ -316,9 +316,7 @@ Position Position::upstream() const
         if (currentNode == enclosingBlock(currentNode) && currentOffset == 0)
             return lastVisible;
             
-        // return position after replaced or BR elements
-        // NOTE: caretMaxOffset() can be less than childNodeCount()!!
-        // e.g. SELECT and APPLET nodes
+        // Return position after brs, tables, and nodes which have content that can be ignored.
         if (editingIgnoresContent(currentNode) || renderer->isBR() || isTableElement(currentNode)) {
             int maxOffset = maxDeepOffset(currentNode);
             if (currentOffset >= maxOffset)
@@ -395,7 +393,7 @@ Position Position::downstream() const
         if (isStreamer(currentPos))
             lastVisible = currentPos;
 
-        // return position before replaced or BR elements
+        // Return position before brs, tables, and nodes which have content that can be ignored.
         if (editingIgnoresContent(currentNode) || renderer->isBR() || isTableElement(currentNode)) {
             if (currentOffset <= renderer->caretMinOffset())
                 return Position(currentNode, renderer->caretMinOffset());
index 72565be68ff68cd3b74668b2077a0f4f84b9cf9e..11e1414048a438a4c216a095b24cc320423e560a 100644 (file)
@@ -80,7 +80,7 @@ void DeleteSelectionCommand::initializeStartEnd()
     Position start = m_selectionToDelete.start();
     Position end = m_selectionToDelete.end();
  
-    // For HR's, we'll get a position at (HR,1) when hitting delete from the beginning of the previous line, or (HR,0) when forward deleting,
+    // For HRs, we'll get a position at (HR,1) when hitting delete from the beginning of the previous line, or (HR,0) when forward deleting,
     // but in these cases, we want to delete it, so manually expand the selection
     if (start.node()->hasTagName(hrTag))
         start = Position(start.node(), 0);
index b107eb63d5c145dd42ec62cd3d0849f5892600fd..33ccf4466a8cd5ecdbbb1fdf8bb65f499913b9f0 100644 (file)
@@ -51,25 +51,11 @@ bool isAtomicNode(const Node *node)
     return node && (!node->hasChildNodes() || editingIgnoresContent(node));
 }
 
-// FIXME: This function needs a comment.
-bool editingIgnoresContent(const Node *node)
+// Returns true for nodes that either have no content, or have content that is ignored (skipped 
+// over) while editing.  There are no VisiblePositions inside these nodes.
+bool editingIgnoresContent(const Node* node)
 {
-    if (!node || !node->isHTMLElement())
-        return false;
-    
-    // There doesn't seem to be a way to find out if a a node is a pop up box by looking at its renderer.
-    if (node->hasTagName(selectTag))
-        return true;
-    
-    if (node->renderer())
-        return node->renderer()->isWidget() || node->renderer()->isImage() || node->renderer()->isHR() || node->renderer()->isTextArea() || node->renderer()->isTextField();
-
-    return node->hasTagName(appletTag) ||
-           node->hasTagName(embedTag) ||
-           node->hasTagName(iframeTag) ||
-           node->hasTagName(imgTag) ||
-           node->hasTagName(hrTag) ||
-           static_cast<const HTMLElement *>(node)->isGenericFormElement();
+    return !canHaveChildrenForEditing(node) && !node->isTextNode();
 }
 
 // Some nodes, like brs, will technically accept children, but we don't want that to happen while editing. 
@@ -83,6 +69,10 @@ bool canHaveChildrenForEditing(const Node* node)
            !node->hasTagName(textareaTag) &&
            !node->hasTagName(objectTag) &&
            !node->hasTagName(iframeTag) &&
+           !node->hasTagName(buttonTag) &&
+           !node->hasTagName(embedTag) &&
+           !node->hasTagName(appletTag) &&
+           !node->hasTagName(selectTag) &&
            !node->isTextNode();
 }
 
index 2de3d0f07e2a3e41c4fd6d011b53ee050c9f24c1..15440fd2b02aac00844ff90e21e6c71996626ac8 100644 (file)
@@ -399,9 +399,9 @@ VisiblePosition previousLinePosition(const VisiblePosition &visiblePosition, int
         // This containing editable block does not have a previous line.
         // Need to move back to previous containing editable block in this root editable
         // block and find the last root line box in that block.
-        Node *startBlock = node->enclosingBlockFlowElement();
+        Node* startBlock = enclosingBlock(node);
         Node *n = node->previousEditable();
-        while (n && startBlock == n->enclosingBlockFlowElement())
+        while (n && startBlock == enclosingBlock(n))
             n = n->previousEditable();
         while (n) {
             if (highestEditableRoot(Position(n, 0)) != highestRoot)
@@ -468,9 +468,9 @@ VisiblePosition nextLinePosition(const VisiblePosition &visiblePosition, int x)
         // This containing editable block does not have a next line.
         // Need to move forward to next containing editable block in this root editable
         // block and find the first root line box in that block.
-        Node *startBlock = node->enclosingBlockFlowElement();
+        Node* startBlock = enclosingBlock(node);
         Node *n = node->nextEditable(p.offset());
-        while (n && startBlock == n->enclosingBlockFlowElement())
+        while (n && startBlock == enclosingBlock(n))
             n = n->nextEditable();
         while (n) {
             if (highestEditableRoot(Position(n, 0)) != highestRoot)