LayoutTests:

Reviewed by darin <rdar://problem/5002441> Pressing space key does nothing above quoted content or a signature Demonstrates the bug: * editing/inserting/5002441-expected.checksum: Added. * editing/inserting/5002441-expected.png: Added. * editing/inserting/5002441-expected.txt: Added. * editing/inserting/5002441.html: Added. Fixed. Spaces passed to execCommand("InsertText", ...) are no longer collapsed: * editing/inserting/editable-html-element-expected.checksum: * editing/inserting/editable-html-element-expected.png: * editing/inserting/editable-html-element-expected.txt: * editing/pasteboard/4989774-expected.checksum: * editing/pasteboard/4989774-expected.png: * editing/pasteboard/4989774-expected.txt: * editing/selection/4983858-expected.checksum: * editing/selection/4983858-expected.png: * editing/selection/4983858-expected.txt: WebCore: Reviewed by darin <rdar://problem/5002441> Pressing space key does nothing above quoted content or a signature Inserting a space under these circumstances inserts a single text node containing a regular space and then does a layout. That space isn't rendered (which is correct). Whitespace rebalancing is supposed to correct it but failed. It replaces the space with a non-breaking space, but that change doesn't dirty line boxes (9441) and so the space isn't rendered. This workaround turns all incoming spaces into non-breaking spaces before they're inserted (they're rebalanced after insertion and turned back into regular spaces if possible). * editing/InsertTextCommand.cpp: (WebCore::InsertTextCommand::prepareForTextInsertion): Removed an old irrelevant FIXME. (WebCore::InsertTextCommand::input): Turn incoming spaces into non breaking spaces before inserting them. Canonical link: https://commits.webkit.org/17713@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@21212 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 1, 2007 · 205caec · 205caec
1 parent a6e3c97
commit 205caec
Show file tree

Hide file tree

Showing 16 changed files with 98 additions and 23 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,28 @@
+2007-05-01  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+
+        <rdar://problem/5002441> 
+        Pressing space key does nothing above quoted content or a signature
+
+        Demonstrates the bug:
+        * editing/inserting/5002441-expected.checksum: Added.
+        * editing/inserting/5002441-expected.png: Added.
+        * editing/inserting/5002441-expected.txt: Added.
+        * editing/inserting/5002441.html: Added.
+
+        Fixed.  Spaces passed to execCommand("InsertText", ...)
+        are no longer collapsed:
+        * editing/inserting/editable-html-element-expected.checksum:
+        * editing/inserting/editable-html-element-expected.png:
+        * editing/inserting/editable-html-element-expected.txt:
+        * editing/pasteboard/4989774-expected.checksum:
+        * editing/pasteboard/4989774-expected.png:
+        * editing/pasteboard/4989774-expected.txt:
+        * editing/selection/4983858-expected.checksum:
+        * editing/selection/4983858-expected.png:
+        * editing/selection/4983858-expected.txt:
+
 2007-05-01  Darin Adler  <darin@apple.com>
 
         Reviewed by Hyatt.

diff --git a/LayoutTests/editing/inserting/5002441-expected.checksum b/LayoutTests/editing/inserting/5002441-expected.checksum
@@ -0,0 +1 @@
+8b38b5e23cc2dd94143e16a399ebda26
diff --git a/LayoutTests/editing/inserting/5002441-expected.png b/LayoutTests/editing/inserting/5002441-expected.png
diff --git a/LayoutTests/editing/inserting/5002441-expected.txt b/LayoutTests/editing/inserting/5002441-expected.txt
@@ -0,0 +1,17 @@
+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 521x18
+          text run at (0,0) width 521: "This tests for a bug where spaces couldn't be inserted before signatures and replies."
+      RenderBlock {DIV} at (0,34) size 784x36
+        RenderBlock (anonymous) at (0,0) size 784x18
+          RenderText {#text} at (0,0) size 4x18
+            text run at (0,0) width 4: " "
+          RenderBR {BR} at (4,14) size 0x0
+        RenderBlock {DIV} at (0,18) size 784x18
+          RenderText {#text} at (0,0) size 345x18
+            text run at (0,0) width 345: "There should be a single space in the paragraph above."
+caret: position 1 of child 0 {#text} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/inserting/5002441.html b/LayoutTests/editing/inserting/5002441.html
@@ -0,0 +1,8 @@
+<p>This tests for a bug where spaces couldn't be inserted before signatures and replies.</p>
+<div id="div" contenteditable="true"><br><div>There should be a single space in the paragraph above.</div></div>
+<script>
+var div = document.getElementById("div");
+var selection = window.getSelection();
+selection.setPosition(div, 0);
+document.execCommand("InsertText", false, " ");
+</script>
diff --git a/LayoutTests/editing/inserting/editable-html-element-expected.checksum b/LayoutTests/editing/inserting/editable-html-element-expected.checksum
@@ -1 +1 @@
-d53960e87b7f062db7c195bd7a7a7f9a
+1186e4514352fbbb37f6518c9093f1c3
diff --git a/LayoutTests/editing/inserting/editable-html-element-expected.png b/LayoutTests/editing/inserting/editable-html-element-expected.png
diff --git a/LayoutTests/editing/inserting/editable-html-element-expected.txt b/LayoutTests/editing/inserting/editable-html-element-expected.txt
@@ -17,9 +17,9 @@ layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x584
       RenderBlock (anonymous) at (0,0) size 784x54
-        RenderText {#text} at (0,0) size 783x54
-          text run at (0,0) width 714: "This tests to make sure that when the enclosing block is the body element, and when the html element is editable, "
-          text run at (714,0) width 69: "inserting a "
+        RenderText {#text} at (0,0) size 784x54
+          text run at (0,0) width 783: "This tests to make sure that when the enclosing block is the body element, and when the html element is editable,  inserting a"
+          text run at (783,0) width 1: " "
           text run at (0,18) width 755: "paragraph separator doesn't split the body (inserting a paragraph separator usually splits/clones the enclosing block flow "
           text run at (0,36) width 58: "element)."
       RenderBlock {DIV} at (0,54) size 784x18

diff --git a/LayoutTests/editing/pasteboard/4989774-expected.checksum b/LayoutTests/editing/pasteboard/4989774-expected.checksum
@@ -1 +1 @@
-6c03d26dbbfba19f07b9a4e4a1c322d0
+be6fd52ad002752178c046831f449f1c
diff --git a/LayoutTests/editing/pasteboard/4989774-expected.png b/LayoutTests/editing/pasteboard/4989774-expected.png
diff --git a/LayoutTests/editing/pasteboard/4989774-expected.txt b/LayoutTests/editing/pasteboard/4989774-expected.txt
@@ -7,11 +7,9 @@ layer at (0,0) size 800x600
       RenderImage {IMG} at (76,0) size 76x103
       RenderImage {IMG} at (152,0) size 76x103
       RenderBR {BR} at (228,103) size 0x0
-      RenderText {#text} at (0,103) size 784x36
-        text run at (0,103) width 629: "This tests for a bug where an images pasted on the same line would appear on different lines. "
-        text run at (629,103) width 153: "You should see several"
-        text run at (782,103) width 2: " "
-        text run at (0,121) width 307: "pictures above all in the same line/paragraph."
+      RenderText {#text} at (0,103) size 739x36
+        text run at (0,103) width 739: "This tests for a bug where an images pasted on the same line would appear on different lines.  You should see "
+        text run at (0,121) width 358: "several pictures above all in the same line/paragraph."
       RenderText {#text} at (0,0) size 0x0
       RenderText {#text} at (0,0) size 0x0
 caret: position 164 of child 4 {#text} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/selection/4983858-expected.checksum b/LayoutTests/editing/selection/4983858-expected.checksum
@@ -1 +1 @@
-27a07777535624367be11a3a86abf096
+28b7b7fc983f2d802da039a49859f928
diff --git a/LayoutTests/editing/selection/4983858-expected.png b/LayoutTests/editing/selection/4983858-expected.png
diff --git a/LayoutTests/editing/selection/4983858-expected.txt b/LayoutTests/editing/selection/4983858-expected.txt
@@ -4,10 +4,8 @@ layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x584
       RenderBlock (anonymous) at (0,0) size 784x36
-        RenderText {#text} at (0,0) size 772x36
-          text run at (0,0) width 570: "This tests for a bug where selecting a word would select the line break and word before it. "
-          text run at (570,0) width 135: "Only the word in the "
-          text run at (705,0) width 67: "paragraph "
+        RenderText {#text} at (0,0) size 780x36
+          text run at (0,0) width 780: "This tests for a bug where selecting a word would select the line break and word before it.  Only the word in the  paragraph "
           text run at (0,18) width 162: "below should be selected:"
       RenderBlock {DIV} at (0,36) size 784x18
         RenderText {#text} at (0,0) size 21x18

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2007-05-01  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+
+        <rdar://problem/5002441> 
+        Pressing space key does nothing above quoted content or a signature
+
+        Inserting a space under these circumstances inserts a single 
+        text node containing a regular space and then does a layout. 
+        That space isn't rendered (which is correct).  Whitespace 
+        rebalancing is supposed to correct it but failed.  It replaces 
+        the space with a non-breaking space, but that change doesn't 
+        dirty line boxes (9441) and so the space isn't rendered.
+
+        This workaround turns all incoming spaces into non-breaking 
+        spaces before they're inserted (they're rebalanced after 
+        insertion and turned back into regular spaces if possible).
+
+        * editing/InsertTextCommand.cpp:
+        (WebCore::InsertTextCommand::prepareForTextInsertion): Removed
+        an old irrelevant FIXME.
+        (WebCore::InsertTextCommand::input): Turn incoming spaces into
+        non breaking spaces before inserting them.
+
 2007-05-01  Darin Adler  <darin@apple.com>
 
         Reviewed by Hyatt.

diff --git a/WebCore/editing/InsertTextCommand.cpp b/WebCore/editing/InsertTextCommand.cpp
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "InsertTextCommand.h"
 
+#include "CharacterNames.h"
 #include "CSSMutableStyleDeclaration.h"
 #include "CSSComputedStyleDeclaration.h"
 #include "Document.h"
@@ -63,15 +64,9 @@ Position InsertTextCommand::prepareForTextInsertion(const Position& p)
     }
     // Prepare for text input by looking at the specified position.
     // It may be necessary to insert a text node to receive characters.
-    // FIXME: What is the rootEditable() check about?  Seems like it
-    // assumes that the content before (or after) pos.node() is editable
-    // (i.e. pos is at an editable/non-editable boundary).  That seems
-    // like a bad assumption.
     if (!pos.node()->isTextNode()) {
         RefPtr<Node> textNode = document()->createEditingTextNode("");
-
         insertNodeAt(textNode.get(), pos);
-
         return Position(textNode.get(), 0);
     }
 
@@ -84,12 +79,21 @@ Position InsertTextCommand::prepareForTextInsertion(const Position& p)
     return pos;
 }
 
-void InsertTextCommand::input(const String &text, bool selectInsertedText)
+void InsertTextCommand::input(const String& originalText, bool selectInsertedText)
 {
+    String text = originalText;
+
     ASSERT(text.find('\n') == -1);
 
     if (endingSelection().isNone())
         return;
+
+    if (RenderObject* renderer = endingSelection().start().node()->renderer())
+        if (renderer->style()->collapseWhiteSpace())
+            // Turn all spaces into non breaking spaces, to make sure that they are treated
+            // literally, and aren't collapsed after insertion. They will be rebalanced 
+            // (turned into a sequence of regular and non breaking spaces) below.
+            text.replace(' ', noBreakSpace);
 
     // Delete the current selection.
     // FIXME: This delete operation blows away the typing style.
@@ -127,7 +131,7 @@ void InsertTextCommand::input(const String &text, bool selectInsertedText)
         // The insertion may require adjusting adjacent whitespace, if it is present.
         rebalanceWhitespaceAt(endPosition);
         // Rebalancing on both sides isn't necessary if we've inserted a space.
-        if (text != " ") 
+        if (originalText != " ") 
             rebalanceWhitespaceAt(startPosition);
 
         m_charactersAdded += text.length();