Reviewed by Ken.

	<rdar://problem/3965466> editing needs to insert text before applying typing style

        * khtml/editing/htmlediting.cpp:
        (khtml::ApplyStyleCommand::addInlineStyleIfNeeded): Remove a FIXME comment.
        (khtml::InsertLineBreakCommand::doApply): Apply style to the BR
	node after inserting it.
        (khtml::InsertTextCommand::prepareForTextInsertion): Don't try to
	apply style to any new nodes created before inserting.
        (khtml::InsertTextCommand::input): Apply style to the inserted range
	after doing the text insert.
        * khtml/editing/htmlediting.h:

	This change leads to improved results on one of the layout tests:

	* layout-tests/editing/inserting/insert-div-024-expected.txt:

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

diff --git a/LayoutTests/editing/inserting/insert-div-024-expected.txt b/LayoutTests/editing/inserting/insert-div-024-expected.txt
index eded054cb3479c31cc4b221e46f79be63e40a82d..60cf41dcd241e5264247d8ef3006223eff8c6344 100644 (file)
--- a/LayoutTests/editing/inserting/insert-div-024-expected.txt
+++ b/LayoutTests/editing/inserting/insert-div-024-expected.txt
@@ -19,10 +19,8 @@ layer at (0,0) size 800x600
           RenderText {TEXT} at (14,14) size 56x28
             text run at (14,14) width 56: "line 1"
       RenderBlock {P} at (0,228) size 784x56 [border: (2px solid #0000FF)]
-        RenderInline {B} at (0,0) size 36x28
-          RenderInline {SPAN} at (0,0) size 36x28
-            RenderText {TEXT} at (14,14) size 36x28
-              text run at (14,14) width 36: "xxx"
+        RenderText {TEXT} at (14,14) size 36x28
+          text run at (14,14) width 36: "xxx"
       RenderBlock {P} at (0,308) size 784x56 [border: (2px solid #0000FF)]
         RenderBR {BR} at (14,14) size 0x28
       RenderBlock {P} at (0,388) size 784x56 [border: (2px solid #0000FF)]

diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23
index 7b6fabdf61e2540bb6d44aaf51cc0518578dd41d..d910ecb3c54e60f0effc718bd11480cca8b9a989 100644 (file)
--- a/WebCore/ChangeLog-2005-08-23
+++ b/WebCore/ChangeLog-2005-08-23
@@ -1,3 +1,23 @@
+2005-01-20  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Ken.
+
+       <rdar://problem/3965466> editing needs to insert text before applying typing style
+       
+        * khtml/editing/htmlediting.cpp:
+        (khtml::ApplyStyleCommand::addInlineStyleIfNeeded): Remove a FIXME comment.
+        (khtml::InsertLineBreakCommand::doApply): Apply style to the BR
+       node after inserting it.
+        (khtml::InsertTextCommand::prepareForTextInsertion): Don't try to
+       apply style to any new nodes created before inserting.
+        (khtml::InsertTextCommand::input): Apply style to the inserted range
+       after doing the text insert.
+        * khtml/editing/htmlediting.h:
+
+       This change leads to improved results on one of the layout tests:
+        
+       * layout-tests/editing/inserting/insert-div-024-expected.txt:
+
 === Safari-180 ===
 
 2005-01-20  Ken Kocienda  <kocienda@apple.com>

diff --git a/WebCore/khtml/editing/htmlediting.cpp b/WebCore/khtml/editing/htmlediting.cpp
index a6d5906d7c3916f87daceb5d37c177572e9f28fa..174269662f3ca0372e9efc412e5d7ca095b60115 100644 (file)
--- a/WebCore/khtml/editing/htmlediting.cpp
+++ b/WebCore/khtml/editing/htmlediting.cpp
@@ -812,54 +812,6 @@ void CompositeEditCommand::rebalanceWhitespace()
     }
 }
 
-NodeImpl *CompositeEditCommand::applyTypingStyle(NodeImpl *child) const
-{
-    // FIXME: This function should share code with ApplyStyleCommand::applyStyleIfNeeded
-    // and ApplyStyleCommand::computeStyleChange.
-    // Both function do similar work, and the common parts could be factored out.
-
-    // FIXME: Improve typing style.
-    // See this bug: <rdar://problem/3769899> Implementation of typing style needs improvement
-
-    // update document layout once before running the rest of the function
-    // so that we avoid the expense of updating before each and every call
-    // to check a computed style
-    document()->updateLayout();
-
-    StyleChange styleChange(document()->part()->typingStyle());
-
-    NodeImpl *childToAppend = child;
-    int exceptionCode = 0;
-
-    if (styleChange.applyItalic()) {
-        ElementImpl *italicElement = document()->createHTMLElement("I", exceptionCode);
-        ASSERT(exceptionCode == 0);
-        italicElement->appendChild(childToAppend, exceptionCode);
-        ASSERT(exceptionCode == 0);
-        childToAppend = italicElement;
-    }
-
-    if (styleChange.applyBold()) {
-        ElementImpl *boldElement = document()->createHTMLElement("B", exceptionCode);
-        ASSERT(exceptionCode == 0);
-        boldElement->appendChild(childToAppend, exceptionCode);
-        ASSERT(exceptionCode == 0);
-        childToAppend = boldElement;
-    }
-
-    if (styleChange.cssStyle().length() > 0) {
-        ElementImpl *styleElement = document()->createHTMLElement("SPAN", exceptionCode);
-        ASSERT(exceptionCode == 0);
-        styleElement->setAttribute(ATTR_STYLE, styleChange.cssStyle());
-        styleElement->setAttribute(ATTR_CLASS, styleSpanClassString());
-        styleElement->appendChild(childToAppend, exceptionCode);
-        ASSERT(exceptionCode == 0);
-        childToAppend = styleElement;
-    }
-
-    return childToAppend;
-}
-
 void CompositeEditCommand::deleteInsignificantText(TextImpl *textNode, int start, int end)
 {
     if (!textNode || !textNode->renderer() || start >= end)
@@ -1604,9 +1556,6 @@ void ApplyStyleCommand::addBlockStyleIfNeeded(CSSMutableStyleDeclarationImpl *st
 
 void ApplyStyleCommand::addInlineStyleIfNeeded(CSSMutableStyleDeclarationImpl *style, NodeImpl *startNode, NodeImpl *endNode)
 {
-    // FIXME: This function should share code with CompositeEditCommand::applyTypingStyle.
-    // Both functions do similar work, and the common parts could be factored out.
-
     StyleChange styleChange(style, Position(startNode, 0));
     int exceptionCode = 0;
     
@@ -2420,13 +2369,6 @@ void InsertLineBreakCommand::doApply()
     ElementImpl *breakNode = createBreakElement(document());
     NodeImpl *nodeToInsert = breakNode;
     
-    // Handle the case where there is a typing style.
-    // FIXME: Improve typing style.
-    // See this bug: <rdar://problem/3769899> Implementation of typing style needs improvement
-    CSSMutableStyleDeclarationImpl *typingStyle = document()->part()->typingStyle();
-    if (typingStyle && typingStyle->length() > 0)
-        nodeToInsert = applyTypingStyle(breakNode);
-    
     Position pos(selection.start().upstream(StayInBlock));
     bool atStart = pos.offset() <= pos.node()->caretMinOffset();
     bool atEnd = pos.offset() >= pos.node()->caretMaxOffset();
@@ -2496,6 +2438,26 @@ void InsertLineBreakCommand::doApply()
         
         setEndingSelection(endingPosition);
     }
+
+    // Handle the case where there is a typing style.
+    // FIXME: Improve typing style.
+    // See this bug: <rdar://problem/3769899> Implementation of typing style needs improvement
+    
+    CSSMutableStyleDeclarationImpl *typingStyle = document()->part()->typingStyle();
+    
+    if (typingStyle && typingStyle->length() > 0) {
+        Selection selectionBeforeStyle = endingSelection();
+
+        DOM::RangeImpl *rangeAroundNode = document()->createRange();
+        int exception;
+        rangeAroundNode->selectNode(nodeToInsert, exception);
+
+        setEndingSelection(Selection(rangeAroundNode));
+        applyStyle(typingStyle);
+
+        setEndingSelection(selectionBeforeStyle);
+    }
+
     rebalanceWhitespace();
 }
 
@@ -2980,17 +2942,12 @@ Position InsertTextCommand::prepareForTextInsertion(bool adjustDownstream)
     else
         pos = pos.upstream(StayInBlock);
     
+    Selection typingStyleRange;
+
     if (!pos.node()->isTextNode()) {
         NodeImpl *textNode = document()->createEditingTextNode("");
         NodeImpl *nodeToInsert = textNode;
 
-        // Handle the case where there is a typing style.
-        // FIXME: Improve typing style.
-        // See this bug: <rdar://problem/3769899> Implementation of typing style needs improvement
-        CSSMutableStyleDeclarationImpl *typingStyle = document()->part()->typingStyle();
-        if (typingStyle && typingStyle->length() > 0)
-            nodeToInsert = applyTypingStyle(textNode);
-        
         // Now insert the node in the right place
         if (pos.node()->isEditableBlock()) {
             LOG(Editing, "prepareForTextInsertion case 1");
@@ -3027,9 +2984,9 @@ Position InsertTextCommand::prepareForTextInsertion(bool adjustDownstream)
             TextImpl *editingTextNode = document()->createEditingTextNode("");
             NodeImpl *node = endingSelection().start().upstream(StayInBlock).node();
             if (node->isBlockFlow())
-                insertNodeAt(applyTypingStyle(editingTextNode), node, 0);
+                insertNodeAt(editingTextNode, node, 0);
             else
-                insertNodeAfter(applyTypingStyle(editingTextNode), node);
+                insertNodeAfter(editingTextNode, node);
             pos = Position(editingTextNode, 0);
         }
     }
@@ -3050,10 +3007,12 @@ void InsertTextCommand::input(const DOMString &text, bool selectInsertedText)
     deleteInsignificantTextDownstream(endingSelection().end().trailingWhitespacePosition());
     
     // Make sure the document is set up to receive text
-    Position pos = prepareForTextInsertion(adjustDownstream);
+    Position startPosition = prepareForTextInsertion(adjustDownstream);
     
-    TextImpl *textNode = static_cast<TextImpl *>(pos.node());
-    long offset = pos.offset();
+    Position endPosition;
+
+    TextImpl *textNode = static_cast<TextImpl *>(startPosition.node());
+    long offset = startPosition.offset();
 
     // Now that we are about to add content, check to see if a placeholder element
     // can be removed.
@@ -3072,18 +3031,15 @@ void InsertTextCommand::input(const DOMString &text, bool selectInsertedText)
             rebalanceWhitespace();
             document()->updateLayout();
         }
-        if (selectInsertedText)
-            setEndingSelection(Selection(Position(textNode, offset), Position(textNode, offset + spacesPerTab)));
-        else
-            setEndingSelection(Position(textNode, offset + spacesPerTab));
+        
+        endPosition = Position(textNode, offset + spacesPerTab);
+
         m_charactersAdded += spacesPerTab;
     }
     else if (isWS(text)) {
         insertSpace(textNode, offset);
-        if (selectInsertedText)
-            setEndingSelection(Selection(Position(textNode, offset), Position(textNode, offset + 1)));
-        else
-            setEndingSelection(Position(textNode, offset + 1));
+        endPosition = Position(textNode, offset + 1);
+
         m_charactersAdded++;
         rebalanceWhitespace();
     }
@@ -3099,12 +3055,22 @@ void InsertTextCommand::input(const DOMString &text, bool selectInsertedText)
             replaceTextInNode(textNode, offset - 1, 1, " ");
         }
         insertTextIntoNode(textNode, offset, text);
-        if (selectInsertedText)
-            setEndingSelection(Selection(Position(textNode, offset), Position(textNode, offset + text.length())));
-        else
-            setEndingSelection(Position(textNode, offset + text.length()));
+        endPosition = Position(textNode, offset + text.length());
+
         m_charactersAdded += text.length();
     }
+
+    setEndingSelection(Selection(startPosition, endPosition));
+
+    // Handle the case where there is a typing style.
+    // FIXME: Improve typing style.
+    // See this bug: <rdar://problem/3769899> Implementation of typing style needs improvement
+    CSSMutableStyleDeclarationImpl *typingStyle = document()->part()->typingStyle();
+    if (typingStyle && typingStyle->length() > 0)
+        applyStyle(typingStyle);
+
+    if (!selectInsertedText)
+        setEndingSelection(endingSelection().end());
 }
 
 void InsertTextCommand::insertSpace(TextImpl *textNode, unsigned long offset)

diff --git a/WebCore/khtml/editing/htmlediting.h b/WebCore/khtml/editing/htmlediting.h
index 71ff0c17a60d7ff038619c204734dfacc88684a8..83f1fcc900593550895863f38133a40e39fd4eaf 100644 (file)
--- a/WebCore/khtml/editing/htmlediting.h
+++ b/WebCore/khtml/editing/htmlediting.h
@@ -212,8 +212,6 @@ protected:
     void setNodeAttribute(DOM::ElementImpl *, int attribute, const DOM::DOMString &);
     void splitTextNode(DOM::TextImpl *text, long offset);
 
-    DOM::NodeImpl *applyTypingStyle(DOM::NodeImpl *) const;
-
     void deleteInsignificantText(DOM::TextImpl *, int start, int end);
     void deleteInsignificantText(const DOM::Position &start, const DOM::Position &end);
     void deleteInsignificantTextDownstream(const DOM::Position &);