Fixed: <rdar://problem/3692199> 8A146: Safari crashes in toHTMLWithOptions, selectio...
authorcblu <cblu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Aug 2004 18:19:40 +0000 (18:19 +0000)
committercblu <cblu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Aug 2004 18:19:40 +0000 (18:19 +0000)
        Reviewed by trey.

        * khtml/xml/dom2_rangeimpl.cpp:
        (DOM::RangeImpl::toHTML): renamed, don't assume that nodes of the range had renderers, use the common ancestor of the range as the root
        * khtml/xml/dom2_rangeimpl.h:
        * khtml/xml/dom_nodeimpl.cpp:
        (NodeImpl::recursive_toHTML): renamed, removed code that determines whether to include the root in the HTML, leave this up to the caller
        * khtml/xml/dom_nodeimpl.h:
        * kwq/WebCoreBridge.mm:
        (-[WebCoreBridge markupStringFromNode:nodes:]): call renamed methods
        (-[WebCoreBridge markupStringFromRange:nodes:]): ditto

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@7287 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
WebCore/khtml/xml/dom_nodeimpl.h
WebCore/kwq/WebCoreBridge.mm

index 7ba7b898c64728449ac60706f053b1d86cbfcc7d..5a718cb2487afd42e9ac358928065cbda56c10bd 100644 (file)
@@ -1,3 +1,19 @@
+2004-08-18  Chris Blumenberg  <cblu@apple.com>
+
+       Fixed: <rdar://problem/3692199> 8A146: Safari crashes in toHTMLWithOptions, selection with no renderer (various sites)
+
+        Reviewed by trey.
+
+        * khtml/xml/dom2_rangeimpl.cpp:
+        (DOM::RangeImpl::toHTML): renamed, don't assume that nodes of the range had renderers, use the common ancestor of the range as the root
+        * khtml/xml/dom2_rangeimpl.h:
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeImpl::recursive_toHTML): renamed, removed code that determines whether to include the root in the HTML, leave this up to the caller
+        * khtml/xml/dom_nodeimpl.h:
+        * kwq/WebCoreBridge.mm:
+        (-[WebCoreBridge markupStringFromNode:nodes:]): call renamed methods
+        (-[WebCoreBridge markupStringFromRange:nodes:]): ditto
+
 2004-08-18  Ken Kocienda  <kocienda@apple.com>
 
         Reviewed by me
index 4f96ec4e60245b4ae088a6009a75a65e4378b38b..4248777a8e635af00db8283d42801b21eaaaa59e 100644 (file)
 #include "dom_textimpl.h"
 #include "dom_xmlimpl.h"
 #include "html/html_elementimpl.h"
+#include "misc/htmltags.h"
 #include "misc/khtml_text_operations.h"
 
 #include "render_block.h"
 
 using khtml::RenderBlock;
+using khtml::RenderObject;
 
 namespace DOM {
 
@@ -829,41 +831,26 @@ DOMString RangeImpl::toString( int &exceptioncode )
     return text;
 }
 
-DOMString RangeImpl::toHTMLWithOptions(QPtrList<NodeImpl> *nodes)
+DOMString RangeImpl::toHTML(QPtrList<NodeImpl> *nodes)
 {
-    // Find the common containing block node of the start and end nodes.
-    RenderBlock *startBlock = m_startContainer->renderer()->containingBlock();
-    RenderBlock *endBlock = m_endContainer->renderer()->containingBlock();
-    NodeImpl *commonBlockNode = 0;
-    while (1) {
-        RenderBlock *newEndBlock = endBlock;
-        while (1) {
-            if (startBlock == newEndBlock) {
-                commonBlockNode = startBlock->element();
-                break;
+    int exceptionCode;
+    NodeImpl *commonAncestor = commonAncestorContainer(exceptionCode);
+    if (commonAncestor == 0) {
+        return "";
+    }
+    RenderObject *renderer = commonAncestor->renderer();
+    if (renderer && !renderer->isRenderBlock()) {
+        RenderBlock *block = renderer->containingBlock();
+        if (block) {
+            NodeImpl *blockElement = block->element();
+            if (blockElement) {
+                commonAncestor = blockElement;
             }
-            if (newEndBlock->isRoot()) {
-                break;
-            }
-            newEndBlock = newEndBlock->containingBlock();
-        }
-        if (commonBlockNode) {
-            break;
-        }
-        RenderBlock *newStartBlock = startBlock->containingBlock();
-        if (!newStartBlock || newStartBlock == startBlock) {
-            commonBlockNode = startBlock->element();
-            break;
         }
-        startBlock = newStartBlock;
     }
-    
-    return commonBlockNode->recursive_toHTMLWithOptions(true, this, nodes);
-}
-
-DOMString RangeImpl::toHTML(  )
-{
-    return toHTMLWithOptions();
+    NodeImpl::Id id = commonAncestor->id();
+    bool onlyIncludeChildren = (id != ID_TABLE && id != ID_OL && id != ID_UL);
+    return commonAncestor->recursive_toHTML(onlyIncludeChildren, this, nodes);
 }
 
 DOMString RangeImpl::text() const
index f7832f29ee41890a7431b384cf03178de077b6cf..9ba0d6b381e2432706f6e0f509860a8b86e0f58c 100644 (file)
@@ -67,8 +67,7 @@ public:
     DocumentFragmentImpl *cloneContents ( int &exceptioncode );
     void insertNode( NodeImpl *newNode, int &exceptioncode );
     DOMString toString ( int &exceptioncode );
-    DOMString toHTML (  );
-    DOMString toHTMLWithOptions(QPtrList<NodeImpl> *nodes=NULL);
+    DOMString toHTML(QPtrList<NodeImpl> *nodes=NULL);
     DOMString text() const;
 
     DocumentFragmentImpl *createContextualFragment ( DOMString &html, int &exceptioncode );
index e61706d65ce8ee3a8f26c2fbcf993d5a1055dfdf..b7a3c2de4948cf83f1e942b5af843f55c129a47b 100644 (file)
@@ -283,23 +283,25 @@ static QString escapeHTML( const QString& in )
     return s;
 }
 
-QString NodeImpl::recursive_toHTMLWithOptions(bool start, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes) const
+QString NodeImpl::recursive_toHTML(bool onlyIncludeChildren, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes) const
 {      
-    bool isNodeIncluded = range ? false : true;
-    Id ident = id();
+    bool include = true;
     
-    // 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.
+    if (onlyIncludeChildren) {
+        // Don't include the HTML of this node, but include the children in the next recursion.
+        include = false;
+    } else if (range) {
+        // If a range is passed, only include the node and its children's HTML if they are in the range or parents on nodes in the range.
+        include = false;
         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;
+                    include = true;
                     break;
                 }
             }
-            if (isNodeIncluded) {
+            if (include) {
                 break;
             }
         }
@@ -307,7 +309,7 @@ QString NodeImpl::recursive_toHTMLWithOptions(bool start, const DOM::RangeImpl *
     
     QString me = "";
     
-    if (isNodeIncluded) {
+    if (include) {
         if (nodes) {
             nodes->append(this);
         }
@@ -342,29 +344,24 @@ QString NodeImpl::recursive_toHTMLWithOptions(bool start, const DOM::RangeImpl *
         }
     }
     
-    if (!isHTMLElement() || endTag[ident] != FORBIDDEN) {
+    if (!isHTMLElement() || endTag[id()] != FORBIDDEN) {
         // print firstChild
         if (NodeImpl *n = firstChild()) {
-            me += n->recursive_toHTMLWithOptions(false, range, nodes);
+            me += n->recursive_toHTML(false, range, nodes);
         }
         // Print my ending tag
-        if (isNodeIncluded && nodeType() != Node::TEXT_NODE && nodeType() != Node::DOCUMENT_NODE) {
+        if (include && nodeType() != Node::TEXT_NODE && nodeType() != Node::DOCUMENT_NODE) {
             me += "</" + nodeName().string() + ">";
         }
     }
     // print next sibling
     if (NodeImpl *n = nextSibling()) {
-        me += n->recursive_toHTMLWithOptions(false, range, nodes);
+        me += n->recursive_toHTML(false, range, nodes);
     }
     
     return me;
 }
 
-QString NodeImpl::recursive_toHTML(bool start) const
-{
-    return recursive_toHTMLWithOptions(start);
-}
-
 void NodeImpl::recursive_completeURLs(QString baseURL)
 {
     if (nodeType() == Node::ELEMENT_NODE) {
index 7a3a62ded962d98d4b1926928fe19a3f190306f9..6af67086da33d9df1b07928d7a6a2ff7aa406e27 100644 (file)
@@ -260,8 +260,7 @@ public:
     
     virtual bool isInline() const;
     virtual QString toHTML() const;
-    QString recursive_toHTML(bool start = false) const;
-    QString recursive_toHTMLWithOptions(bool start=false, const DOM::RangeImpl *range=NULL, QPtrList<NodeImpl> *nodes=NULL) const;
+    QString recursive_toHTML(bool onlyIncludeChildren=false, const DOM::RangeImpl *range=NULL, QPtrList<NodeImpl> *nodes=NULL) const;
     void recursive_completeURLs(QString baseURL);
     
     virtual bool isContentEditable() const;
index 1aca048d385dd0e02ebee68391933e152e560acc..c39dd6004cee59eba28fb76bbeefa9ca534c67f8 100644 (file)
@@ -500,7 +500,7 @@ static bool initializedKJS = FALSE;
     if (nodes) {
         nodeList = new QPtrList<NodeImpl>;
     }
-    NSString *markupString = [node _nodeImpl]->recursive_toHTMLWithOptions(false, NULL, nodeList).getNSString();
+    NSString *markupString = [node _nodeImpl]->recursive_toHTML(false, NULL, nodeList).getNSString();
     if (nodes) {
         *nodes = [self nodesFromList:nodeList];
         delete nodeList;
@@ -514,7 +514,7 @@ static bool initializedKJS = FALSE;
     if (nodes) {
         nodeList = new QPtrList<NodeImpl>;
     }
-    NSString *markupString = [range _rangeImpl]->toHTMLWithOptions(nodeList).string().getNSString();
+    NSString *markupString = [range _rangeImpl]->toHTML(nodeList).string().getNSString();
     if (nodes) {
         *nodes = [self nodesFromList:nodeList];
         delete nodeList;