Reviewed by Ken.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Aug 2004 16:39:08 +0000 (16:39 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Aug 2004 16:39:08 +0000 (16:39 +0000)
        - fixed <rdar://problem/3758756> copying text selected with down arrow results in all text to end of document

        * khtml/xml/dom2_rangeimpl.h: Make startNode and pastEndNode public.
        * khtml/xml/dom2_rangeimpl.cpp: (DOM::RangeImpl::pastEndNode): Fix bug where this would return
        one node too far in the case where the end container was not a text node.

        * khtml/xml/dom_nodeimpl.cpp:
        (NodeImpl::recursive_toHTMLWithOptions): Rewrite loop, using startNode and pastEndNode,
        to fix bug where it would run past the end node, including too many nodes in the generated
        HTML. Nice side benefit: easier to read the code.

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/xml/dom2_rangeimpl.cpp
WebCore/khtml/xml/dom2_rangeimpl.h
WebCore/khtml/xml/dom_nodeimpl.cpp

index b38fc31e1df2a84bac50e5394b36ecc0b6da645c..8cb524da1ded8880b206fef02789096e8863d7c7 100644 (file)
@@ -1,3 +1,18 @@
+2004-08-12  Darin Adler  <darin@apple.com>
+
+        Reviewed by Ken.
+
+        - fixed <rdar://problem/3758756> copying text selected with down arrow results in all text to end of document
+
+        * khtml/xml/dom2_rangeimpl.h: Make startNode and pastEndNode public.
+        * khtml/xml/dom2_rangeimpl.cpp: (DOM::RangeImpl::pastEndNode): Fix bug where this would return
+        one node too far in the case where the end container was not a text node.
+
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeImpl::recursive_toHTMLWithOptions): Rewrite loop, using startNode and pastEndNode,
+        to fix bug where it would run past the end node, including too many nodes in the generated
+        HTML. Nice side benefit: easier to read the code.
+
 2004-08-11  Chris Blumenberg  <cblu@apple.com>
 
        Fixed: <rdar://problem/3758216> PARENTAL: buttons on parental controls page only work once
index 6fe3c32e287a29973ce665aa36eee7aca272d9ee..4f96ec4e60245b4ae088a6009a75a65e4378b38b 100644 (file)
@@ -1352,7 +1352,7 @@ NodeImpl *RangeImpl::pastEndNode() const
     }
     NodeImpl *child = m_endContainer->childNode(m_endOffset);
     if (child)
-        return child->traverseNextSibling();
+        return child;
     return m_endContainer->traverseNextSibling();
 }
 
index 209864f60dde1c41c1d11bdd7fe8ed81fbc2f19d..f7832f29ee41890a7431b384cf03178de077b6cf 100644 (file)
@@ -94,6 +94,9 @@ public:
 
     bool readOnly() { return false; }
 
+    NodeImpl *startNode() const;
+    NodeImpl *pastEndNode() const;
+
 #if APPLE_CHANGES
     static Range createInstance (RangeImpl *impl);
 #endif
@@ -117,9 +120,6 @@ private:
     void setEndContainer(NodeImpl *_endContainer);
     void checkDeleteExtract(int &exceptioncode);
     bool containedByReadOnly() const;
-
-    NodeImpl *startNode() const;
-    NodeImpl *pastEndNode() const;
 };
 
 } // namespace
index b11f8631d946c60cc7bfa8313852d47ed5c8a632..e61706d65ce8ee3a8f26c2fbcf993d5a1055dfdf 100644 (file)
@@ -285,45 +285,28 @@ static QString escapeHTML( const QString& in )
 
 QString NodeImpl::recursive_toHTMLWithOptions(bool start, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes) const
 {      
-    QString me = "";
-    
-    int exceptionCode;
-    NodeImpl *startContainer = range ? range->startContainer(exceptionCode) : NULL;
-    NodeImpl *endContainer = range ? range->endContainer(exceptionCode) : NULL;
-    NodeImpl *n = startContainer;
     bool isNodeIncluded = range ? false : true;
     Id ident = id();
     
     // Determine if the HTML string of this node should be part of the end result.
     if (range && (!start || (start && (ident == ID_TABLE || ident == ID_OL || ident == ID_UL)))) {     
         // Check if this node is in the range or is an ancestor of a node in the range.
-        while (n) {
-            NodeImpl *ancestor = n;
-            while (ancestor) {
+        NodeImpl *pastEnd = range->pastEndNode();
+        for (NodeImpl *n = range->startNode(); n != pastEnd; n = n->traverseNextNode()) {
+            for (NodeImpl *ancestor = n; ancestor; ancestor = ancestor->parentNode()) {
                 if (this == ancestor) {
                     isNodeIncluded = true;
                     break;
                 }
-                ancestor = ancestor->parentNode();
             }
             if (isNodeIncluded) {
                 break;
             }
-            if (n == endContainer) {
-                break;
-            }
-            NodeImpl *next = n->firstChild();
-            if (!next) {
-                next = n->nextSibling();
-            }
-            while (!next && n->parentNode()) {
-                n = n->parentNode();
-                next = n->nextSibling();
-            }
-            n = next;
         }
     }
     
+    QString me = "";
+    
     if (isNodeIncluded) {
         if (nodes) {
             nodes->append(this);
@@ -332,10 +315,11 @@ QString NodeImpl::recursive_toHTMLWithOptions(bool start, const DOM::RangeImpl *
         if (nodeType() == Node::TEXT_NODE) {
             DOMString str = nodeValue().copy();
             if (range) {
-                if (this == endContainer) {
+                int exceptionCode;
+                if (this == range->endContainer(exceptionCode)) {
                     str.truncate(range->endOffset(exceptionCode));
                 }
-                if (this == startContainer) {
+                if (this == range->startContainer(exceptionCode)) {
                     str.remove(0, range->startOffset(exceptionCode));
                 }
             }
@@ -360,7 +344,7 @@ QString NodeImpl::recursive_toHTMLWithOptions(bool start, const DOM::RangeImpl *
     
     if (!isHTMLElement() || endTag[ident] != FORBIDDEN) {
         // print firstChild
-        if ((n = firstChild())) {
+        if (NodeImpl *n = firstChild()) {
             me += n->recursive_toHTMLWithOptions(false, range, nodes);
         }
         // Print my ending tag
@@ -369,7 +353,7 @@ QString NodeImpl::recursive_toHTMLWithOptions(bool start, const DOM::RangeImpl *
         }
     }
     // print next sibling
-    if ((n = nextSibling())) {
+    if (NodeImpl *n = nextSibling()) {
         me += n->recursive_toHTMLWithOptions(false, range, nodes);
     }
     
@@ -1290,7 +1274,11 @@ void NodeImpl::createRendererIfNeeded()
             m_render = createRenderer(getDocument()->renderArena(), style);
             m_render->setStyle(style);
             parentRenderer->addChild(m_render, nextRenderer());
+#ifndef KHTML_NO_XBL
+        } // avoid confusing the change log code parser by having two close braces to match the two open braces above
+#else
         }
+#endif
         style->deref(getDocument()->renderArena());
     }
 }