Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Dec 2004 21:01:02 +0000 (21:01 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Dec 2004 21:01:02 +0000 (21:01 +0000)
        WebCore side of fix for this bug:

        <rdar://problem/3768372> REGRESSION (Mail): paste of text ending in whitespace loses whitespace

        Note that we are coordinating with Doug Davidson on the AppKit team to make a complete fix for this
        bug. This change involves our half of the needed changes.

        Note that a lot of this change has to do with changing code to use a <br> element instead of
        a comment node as the mechanism to annotate HTML with information used to fix the bug. In some
        other places, code to handle comments in markup can be removed since we do not use comments for
        such annotations after this change.

        * khtml/editing/htmlediting.cpp: Remove isComment() helper; no longer needed.
        (khtml::ReplacementFragment::ReplacementFragment): Change m_hasInterchangeNewlineComment name to m_hasInterchangeNewline.
        (khtml::ReplacementFragment::isInterchangeNewlineNode): Name changed from isInterchangeNewlineComment.
        (khtml::ReplacementFragment::isInterchangeConvertedSpaceSpan): Local variable name convertedSpaceSpanClass changed to
        convertedSpaceSpanClassString to match other uses of the idiom used here.
        (khtml::ReplaceSelectionCommand::doApply): Change hasInterchangeNewlineComment() name to hasInterchangeNewline().
        * khtml/editing/htmlediting.h: Change names as noted in .cpp file. Remove isComment() helper; no longer needed.
        (khtml::ReplacementFragment::hasInterchangeNewline):  Change hasInterchangeNewlineComment() name to hasInterchangeNewline().
        * khtml/html/html_elementimpl.cpp:
        (HTMLElementImpl::createContextualFragment): No longer has includeCommentsInDOM flag; no longer needed as we do not
        annotate fragments with comments any longer.
        * khtml/html/html_elementimpl.h: Ditto.
        * khtml/xml/dom2_rangeimpl.cpp: Remove addCommentToHTMLMarkup() helper. No longer needed.
        (DOM::interchangeNewlineMarkupString): New helper to return <br> element markup we use to annotate content for interchange.
        (DOM::RangeImpl::toHTML): No longer uses addCommentToHTMLMarkup; now calls interchangeNewlineMarkupString(). Remove
        spurious semi-colon.
        * khtml/xml/dom2_rangeimpl.h: Remove obsolete addCommentToHTMLMarkup() function and EAddToMarkup enum.
        * kwq/WebCoreBridge.mm:
        (-[WebCoreBridge documentFragmentWithMarkupString:baseURLString:]): No longer pass bool to ask for including comments
        in DOM when calling createContextualFragment().

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/editing/htmlediting.h
WebCore/khtml/html/html_elementimpl.cpp
WebCore/khtml/html/html_elementimpl.h
WebCore/khtml/xml/dom2_rangeimpl.cpp
WebCore/khtml/xml/dom2_rangeimpl.h
WebCore/kwq/WebCoreBridge.mm

index 2fa9e54c4ac2396d546d31fa197d035f55bfd9d7..cc15017038499d88a53c4f143c1b0487b1924539 100644 (file)
@@ -1,3 +1,40 @@
+2004-12-13  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+
+        WebCore side of fix for this bug:
+        
+        <rdar://problem/3768372> REGRESSION (Mail): paste of text ending in whitespace loses whitespace
+
+        Note that we are coordinating with Doug Davidson on the AppKit team to make a complete fix for this
+        bug. This change involves our half of the needed changes.
+        
+        Note that a lot of this change has to do with changing code to use a <br> element instead of
+        a comment node as the mechanism to annotate HTML with information used to fix the bug. In some
+        other places, code to handle comments in markup can be removed since we do not use comments for
+        such annotations after this change.
+
+        * khtml/editing/htmlediting.cpp: Remove isComment() helper; no longer needed.
+        (khtml::ReplacementFragment::ReplacementFragment): Change m_hasInterchangeNewlineComment name to m_hasInterchangeNewline.
+        (khtml::ReplacementFragment::isInterchangeNewlineNode): Name changed from isInterchangeNewlineComment.
+        (khtml::ReplacementFragment::isInterchangeConvertedSpaceSpan): Local variable name convertedSpaceSpanClass changed to
+        convertedSpaceSpanClassString to match other uses of the idiom used here.
+        (khtml::ReplaceSelectionCommand::doApply): Change hasInterchangeNewlineComment() name to hasInterchangeNewline().
+        * khtml/editing/htmlediting.h: Change names as noted in .cpp file. Remove isComment() helper; no longer needed.
+        (khtml::ReplacementFragment::hasInterchangeNewline):  Change hasInterchangeNewlineComment() name to hasInterchangeNewline().
+        * khtml/html/html_elementimpl.cpp:
+        (HTMLElementImpl::createContextualFragment): No longer has includeCommentsInDOM flag; no longer needed as we do not
+        annotate fragments with comments any longer.
+        * khtml/html/html_elementimpl.h: Ditto.
+        * khtml/xml/dom2_rangeimpl.cpp: Remove addCommentToHTMLMarkup() helper. No longer needed.
+        (DOM::interchangeNewlineMarkupString): New helper to return <br> element markup we use to annotate content for interchange.
+        (DOM::RangeImpl::toHTML): No longer uses addCommentToHTMLMarkup; now calls interchangeNewlineMarkupString(). Remove
+        spurious semi-colon.
+        * khtml/xml/dom2_rangeimpl.h: Remove obsolete addCommentToHTMLMarkup() function and EAddToMarkup enum.
+        * kwq/WebCoreBridge.mm:
+        (-[WebCoreBridge documentFragmentWithMarkupString:baseURLString:]): No longer pass bool to ask for including comments
+        in DOM when calling createContextualFragment().
+
 2004-12-10  John Sullivan  <sullivan@apple.com>
 
         fixed deployment build bustage that John Louch ran into
index 8add5802e60fa4c723607fbf80c7b649e69be6fd..419737f34faacb9036c6133c22f397dff719df34 100644 (file)
@@ -3029,7 +3029,7 @@ void RemoveNodePreservingChildrenCommand::doApply()
 // ReplaceSelectionCommand
 
 ReplacementFragment::ReplacementFragment(DocumentFragmentImpl *fragment)
-    : m_fragment(fragment), m_hasInterchangeNewlineComment(false), m_hasMoreThanOneBlock(false)
+    : m_fragment(fragment), m_hasInterchangeNewline(false), m_hasMoreThanOneBlock(false)
 {
     if (!m_fragment) {
         m_type = EmptyFragment;
@@ -3055,12 +3055,12 @@ ReplacementFragment::ReplacementFragment(DocumentFragmentImpl *fragment)
 
     NodeImpl *node = firstChild;
     int realBlockCount = 0;
-    NodeImpl *commentToDelete = 0;
+    NodeImpl *nodeToDelete = 0;
     while (node) {
         NodeImpl *next = node->traverseNextNode();
-        if (isInterchangeNewlineComment(node)) {
-            m_hasInterchangeNewlineComment = true;
-            commentToDelete = node;
+        if (isInterchangeNewlineNode(node)) {
+            m_hasInterchangeNewline = true;
+            nodeToDelete = node;
         }
         else if (isInterchangeConvertedSpaceSpan(node)) {
             NodeImpl *n = 0;
@@ -3079,8 +3079,8 @@ ReplacementFragment::ReplacementFragment(DocumentFragmentImpl *fragment)
         node = next;
     }
 
-    if (commentToDelete)
-        removeNode(commentToDelete);
+    if (nodeToDelete)
+        removeNode(nodeToDelete);
 
     int blockCount = realBlockCount;
     firstChild = m_fragment->firstChild();
@@ -3164,15 +3164,16 @@ void ReplacementFragment::pruneEmptyNodes()
     }
 }
 
-bool ReplacementFragment::isInterchangeNewlineComment(const NodeImpl *node)
+bool ReplacementFragment::isInterchangeNewlineNode(const NodeImpl *node)
 {
-    return isComment(node) && node->nodeValue() == AppleInterchangeNewline;
+    static DOMString interchangeNewlineClassString(AppleInterchangeNewline);
+    return node && node->id() == ID_BR && static_cast<const ElementImpl *>(node)->getAttribute(ATTR_CLASS) == interchangeNewlineClassString;
 }
 
 bool ReplacementFragment::isInterchangeConvertedSpaceSpan(const NodeImpl *node)
 {
-    static DOMString convertedSpaceSpanClass(AppleConvertedSpace);
-    return node->isHTMLElement() && static_cast<const HTMLElementImpl *>(node)->getAttribute(ATTR_CLASS) == convertedSpaceSpanClass;
+    static DOMString convertedSpaceSpanClassString(AppleConvertedSpace);
+    return node->isHTMLElement() && static_cast<const HTMLElementImpl *>(node)->getAttribute(ATTR_CLASS) == convertedSpaceSpanClassString;
 }
 
 void ReplacementFragment::removeNode(NodeImpl *node)
@@ -3204,11 +3205,6 @@ void ReplacementFragment::insertNodeBefore(NodeImpl *node, NodeImpl *refNode)
  }
 
 
-bool isComment(const NodeImpl *node)
-{
-    return node && node->nodeType() == Node::COMMENT_NODE;
-}
-
 bool isProbablyBlock(const NodeImpl *node)
 {
     if (!node)
@@ -3264,15 +3260,15 @@ void ReplaceSelectionCommand::doApply()
     NodeImpl *startBlock = selection.start().node()->enclosingBlockFlowElement();
     NodeImpl *endBlock = selection.end().node()->enclosingBlockFlowElement();
 
-    bool mergeStart = !(startAtStartOfLine && (m_fragment.hasInterchangeNewlineComment() || m_fragment.hasMoreThanOneBlock()));
-    bool mergeEnd = !m_fragment.hasInterchangeNewlineComment() && m_fragment.hasMoreThanOneBlock();
+    bool mergeStart = !(startAtStartOfLine && (m_fragment.hasInterchangeNewline() || m_fragment.hasMoreThanOneBlock()));
+    bool mergeEnd = !m_fragment.hasInterchangeNewline() && m_fragment.hasMoreThanOneBlock();
     Position startPos = Position(selection.start().node()->enclosingBlockFlowElement(), 0);
     Position endPos; 
     EStayInBlock upstreamStayInBlock = StayInBlock;
 
     // Delete the current selection, or collapse whitespace, as needed
     if (selection.isRange()) {
-        deleteSelection(false, !(m_fragment.hasInterchangeNewlineComment() || m_fragment.hasMoreThanOneBlock()));
+        deleteSelection(false, !(m_fragment.hasInterchangeNewline() || m_fragment.hasMoreThanOneBlock()));
     }
     else if (selection.isCaret() && mergeEnd && !startAtBlockBoundary) {
         // The start and the end need to wind up in separate blocks.
@@ -3464,7 +3460,7 @@ void ReplaceSelectionCommand::doApply()
     }
 
     // Handle trailing newline
-    if (m_fragment.hasInterchangeNewlineComment()) {
+    if (m_fragment.hasInterchangeNewline()) {
         if (startBlock == endBlock && !isProbablyBlock(lastNodeInserted)) {
             setEndingSelection(insertionPos);
             insertParagraphSeparator();
index 93c1f5bfa29416b60568fb47ed1f079030abef78..9eeb2a8d0e6f5724a8ce20acfe37eb887b12dbc7 100644 (file)
@@ -630,14 +630,14 @@ public:
     bool isTreeFragment() const { return m_type == TreeFragment; }
 
     bool hasMoreThanOneBlock() const { return m_hasMoreThanOneBlock; }
-    bool hasInterchangeNewlineComment() const { return m_hasInterchangeNewlineComment; }
+    bool hasInterchangeNewline() const { return m_hasInterchangeNewline; }
 
 private:
     // no copy construction or assignment
     ReplacementFragment(const ReplacementFragment &);
     ReplacementFragment &operator=(const ReplacementFragment &);
 
-    static bool isInterchangeNewlineComment(const DOM::NodeImpl *);
+    static bool isInterchangeNewlineNode(const DOM::NodeImpl *);
     static bool isInterchangeConvertedSpaceSpan(const DOM::NodeImpl *);
 
     // A couple simple DOM helpers
@@ -646,13 +646,12 @@ private:
 
     EFragmentType m_type;
     DOM::DocumentFragmentImpl *m_fragment;
-    bool m_hasInterchangeNewlineComment;
+    bool m_hasInterchangeNewline;
     bool m_hasMoreThanOneBlock;
 };
 
 // free-floating helper functions
 bool isProbablyBlock(const DOM::NodeImpl *);
-bool isComment(const DOM::NodeImpl *);
 
 class ReplaceSelectionCommand : public CompositeEditCommand
 {
index 28d76063cdd88744ec6ff278d2300b4c3702f004..3e51e1a8b09ad826b91841de08007f8b17d4822d 100644 (file)
@@ -727,7 +727,7 @@ DOMString HTMLElementImpl::outerText() const
     return innerText();
 }
 
-DocumentFragmentImpl *HTMLElementImpl::createContextualFragment(const DOMString &html, bool includeCommentsInDOM)
+DocumentFragmentImpl *HTMLElementImpl::createContextualFragment(const DOMString &html)
 {
     // the following is in accordance with the definition as used by IE
     if( endTag[id()] == FORBIDDEN )
@@ -752,7 +752,7 @@ DocumentFragmentImpl *HTMLElementImpl::createContextualFragment(const DOMString
     DocumentFragmentImpl *fragment = new DocumentFragmentImpl( docPtr() );
     fragment->ref();
     {
-        HTMLTokenizer tok(docPtr(), fragment, includeCommentsInDOM);
+        HTMLTokenizer tok(docPtr(), fragment);
         tok.write( html.string(), true );
         tok.finish();
     }
index cad95fecdd7da6e4cf7ea5f97539324713ed35de..52bd57a390c604cb1440c98c74ad997b04b4c74f 100644 (file)
@@ -151,7 +151,7 @@ public:
     DOMString outerHTML() const;
     DOMString innerText() const;
     DOMString outerText() const;
-    DocumentFragmentImpl *createContextualFragment(const DOMString &html, bool includeCommentsInDOM=false);
+    DocumentFragmentImpl *createContextualFragment(const DOMString &html);
     bool setInnerHTML( const DOMString &html );
     bool setOuterHTML( const DOMString &html );
     bool setInnerText( const DOMString &text );
index 7c00d35d72d9db7d95bb0bc1f2847b5241bee396..5ba38cb97187035310c52d9f3241049738d2fa1d 100644 (file)
@@ -834,22 +834,15 @@ DOMString RangeImpl::toString( int &exceptioncode ) const
     return text;
 }
 
-void RangeImpl::addCommentToHTMLMarkup(const DOMString &comment, QStringList &markups, EAddToMarkup appendOrPrepend) const
- {
-    if (!m_ownerDocument)
-        return;
-        
-    CommentImpl *n = new CommentImpl(m_ownerDocument, comment);
-    n->ref();
-    switch (appendOrPrepend) {
-        case PrependToMarkup:
-            markups.prepend(n->startMarkup(this));
-            break;
-        case AppendToMarkup:
-            markups.append(n->startMarkup(this));
-            break;
+static QString interchangeNewlineMarkupString()
+{
+    static QString interchangeNewlineString;
+    if (interchangeNewlineString.length() == 0) {
+        interchangeNewlineString = "<br class=\"";
+        interchangeNewlineString += AppleInterchangeNewline;
+        interchangeNewlineString += "\">";
     }
-    n->deref();
+    return interchangeNewlineString;
 }
 
 DOMString RangeImpl::toHTML(QPtrList<NodeImpl> *nodes, EAnnotateForInterchange annotate) const
@@ -954,12 +947,11 @@ DOMString RangeImpl::toHTML(QPtrList<NodeImpl> *nodes, EAnnotateForInterchange a
         Position pos(m_endContainer, m_endOffset);
         NodeImpl *block = pos.node()->enclosingBlockFlowElement();
         NodeImpl *upstreamBlock = pos.upstream().node()->enclosingBlockFlowElement();
-        if (block != upstreamBlock) {
-            addCommentToHTMLMarkup(AppleInterchangeNewline, markups, AppendToMarkup);    
-        }
+        if (block != upstreamBlock)
+            markups.append(interchangeNewlineMarkupString());
     }
     
-    return markups.join("");;
+    return markups.join("");
 }
 
 
index ebc4e998aa2bb0d964f6637b5a1c30adfb8ac3cb..b46419324efe763074b42bb334d8b9f53111d2a6 100644 (file)
@@ -122,9 +122,6 @@ private:
     void setEndContainer(NodeImpl *_endContainer);
     void checkDeleteExtract(int &exceptioncode);
     bool containedByReadOnly() const;
-    
-    enum EAddToMarkup { PrependToMarkup, AppendToMarkup };
-    void addCommentToHTMLMarkup(const DOMString &, QStringList &, EAddToMarkup) const;
 };
 
 } // namespace
index 7dc74deb5a5471c606fa4118668ec5f6c8994f25..338e53e361608c4561a7db96fa1d4642631a563e 100644 (file)
@@ -1552,7 +1552,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 {
     DocumentImpl *document = _part->xmlDocImpl();
     HTMLElementImpl *element = static_cast<HTMLElementImpl *>(document->documentElement());
-    DocumentFragmentImpl *fragment = element->createContextualFragment(markupString, true); // true to include comments in DOM
+    DocumentFragmentImpl *fragment = element->createContextualFragment(markupString);
     ASSERT(fragment);
     
     if ([baseURLString length] > 0) {