Reviewed by Chris.
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Aug 2004 00:31:08 +0000 (00:31 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Aug 2004 00:31:08 +0000 (00:31 +0000)
- fixed <rdar://problem/3778043> REGRESSION: innerHTML is broken, breaks automated iBench testing
- also fixed outerHTML, which would spill over past the node for whch it was supposed to get HTML

        * khtml/html/html_elementimpl.cpp:
        (HTMLElementImpl::outerHTML):
        * khtml/xml/dom2_rangeimpl.cpp:
        (DOM::RangeImpl::toHTML):
        * khtml/xml/dom_nodeimpl.cpp:
        (NodeImpl::toHTML):
        (NodeImpl::recursive_toString):
        (NodeImpl::recursive_toHTML):
        * khtml/xml/dom_nodeimpl.h:
        * kwq/WebCoreBridge.mm:
        (-[WebCoreBridge markupStringFromNode:nodes:]):

- added new layout tests to cover the problems I fixed

* layout-tests/fast/innerHTML/001-expected.txt: Added.
        * layout-tests/fast/innerHTML/001.html: Added.
        * layout-tests/fast/innerHTML/002-expected.txt: Added.
        * layout-tests/fast/innerHTML/002.html: Added.
        * layout-tests/fast/innerHTML/003-expected.txt: Added.
        * layout-tests/fast/innerHTML/003.html: Added.

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

12 files changed:
LayoutTests/fast/innerHTML/001-expected.txt [new file with mode: 0644]
LayoutTests/fast/innerHTML/001.html [new file with mode: 0644]
LayoutTests/fast/innerHTML/002-expected.txt [new file with mode: 0644]
LayoutTests/fast/innerHTML/002.html [new file with mode: 0644]
LayoutTests/fast/innerHTML/003-expected.txt [new file with mode: 0644]
LayoutTests/fast/innerHTML/003.html [new file with mode: 0644]
WebCore/ChangeLog-2005-08-23
WebCore/khtml/html/html_elementimpl.cpp
WebCore/khtml/xml/dom2_rangeimpl.cpp
WebCore/khtml/xml/dom_nodeimpl.cpp
WebCore/khtml/xml/dom_nodeimpl.h
WebCore/kwq/WebCoreBridge.mm

diff --git a/LayoutTests/fast/innerHTML/001-expected.txt b/LayoutTests/fast/innerHTML/001-expected.txt
new file mode 100644 (file)
index 0000000..1215ca5
--- /dev/null
@@ -0,0 +1,11 @@
+layer at (0,0) size 800x600
+  RenderCanvas 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 {DIV} at (0,0) size 784x18
+        RenderText {TEXT} at (0,0) size 191x18
+          text run at (0,0) width 191: "this text should show up twice"
+      RenderBlock {DIV} at (0,18) size 784x18
+        RenderText {TEXT} at (0,0) size 191x18
+          text run at (0,0) width 191: "this text should show up twice"
diff --git a/LayoutTests/fast/innerHTML/001.html b/LayoutTests/fast/innerHTML/001.html
new file mode 100644 (file)
index 0000000..eb51913
--- /dev/null
@@ -0,0 +1,10 @@
+<html>
+<body>
+<div id="x">this text should show up twice</div>
+<div id="y"></div>
+<script language="JavaScript">
+document.getElementById("y").innerHTML = document.getElementById("x").innerHTML;
+</script>
+</body>
+</html>
+
diff --git a/LayoutTests/fast/innerHTML/002-expected.txt b/LayoutTests/fast/innerHTML/002-expected.txt
new file mode 100644 (file)
index 0000000..5b417de
--- /dev/null
@@ -0,0 +1,13 @@
+layer at (0,0) size 800x600
+  RenderCanvas 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 {DIV} at (0,0) size 784x18
+        RenderInline {B} at (0,0) size 230x18
+          RenderText {TEXT} at (0,0) size 230x18
+            text run at (0,0) width 230: "this text should be bold both times"
+      RenderBlock {DIV} at (0,18) size 784x18
+        RenderInline {B} at (0,0) size 230x18
+          RenderText {TEXT} at (0,0) size 230x18
+            text run at (0,0) width 230: "this text should be bold both times"
diff --git a/LayoutTests/fast/innerHTML/002.html b/LayoutTests/fast/innerHTML/002.html
new file mode 100644 (file)
index 0000000..275b490
--- /dev/null
@@ -0,0 +1,10 @@
+<html>
+<body>
+<div id="x"><b>this text should be bold both times</b></div>
+<div id="y"></div>
+<script language="JavaScript">
+document.getElementById("y").innerHTML = document.getElementById("x").innerHTML;
+</script>
+</body>
+</html>
+
diff --git a/LayoutTests/fast/innerHTML/003-expected.txt b/LayoutTests/fast/innerHTML/003-expected.txt
new file mode 100644 (file)
index 0000000..620cd92
--- /dev/null
@@ -0,0 +1,15 @@
+layer at (0,0) size 800x600
+  RenderCanvas 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 {DIV} at (0,0) size 784x18
+        RenderText {TEXT} at (0,0) size 191x18
+          text run at (0,0) width 191: "this text should show up twice"
+      RenderBlock {DIV} at (0,18) size 784x18
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderText {TEXT} at (0,0) size 191x18
+            text run at (0,0) width 191: "this text should show up twice"
+      RenderBlock {DIV} at (0,36) size 784x18
+        RenderText {TEXT} at (0,0) size 187x18
+          text run at (0,0) width 187: "this text should show up once"
diff --git a/LayoutTests/fast/innerHTML/003.html b/LayoutTests/fast/innerHTML/003.html
new file mode 100644 (file)
index 0000000..daff18c
--- /dev/null
@@ -0,0 +1,11 @@
+<html>
+<body>
+<div id="x">this text should show up twice</div>
+<div id="y"></div>
+<div>this text should show up once</div>
+<script language="JavaScript">
+document.getElementById("y").innerHTML = document.getElementById("x").outerHTML;
+</script>
+</body>
+</html>
+
index 2e24eb1..e5ed944 100644 (file)
@@ -1,3 +1,31 @@
+2004-08-27  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Chris.
+
+       - fixed <rdar://problem/3778043> REGRESSION: innerHTML is broken, breaks automated iBench testing
+       - also fixed outerHTML, which would spill over past the node for whch it was supposed to get HTML
+       
+        * khtml/html/html_elementimpl.cpp:
+        (HTMLElementImpl::outerHTML):
+        * khtml/xml/dom2_rangeimpl.cpp:
+        (DOM::RangeImpl::toHTML):
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeImpl::toHTML):
+        (NodeImpl::recursive_toString):
+        (NodeImpl::recursive_toHTML):
+        * khtml/xml/dom_nodeimpl.h:
+        * kwq/WebCoreBridge.mm:
+        (-[WebCoreBridge markupStringFromNode:nodes:]):
+
+       - added new layout tests to cover the problems I fixed
+        
+       * layout-tests/fast/innerHTML/001-expected.txt: Added.
+        * layout-tests/fast/innerHTML/001.html: Added.
+        * layout-tests/fast/innerHTML/002-expected.txt: Added.
+        * layout-tests/fast/innerHTML/002.html: Added.
+        * layout-tests/fast/innerHTML/003-expected.txt: Added.
+        * layout-tests/fast/innerHTML/003.html: Added.
+
 2004-08-27  David Hyatt  <hyatt@apple.com>
 
        Fix for 3739239, getComputedStyle of top not being implemented broke a site that checked for it.
index 8da433f..bf668b0 100644 (file)
@@ -698,7 +698,7 @@ DOMString HTMLElementImpl::innerHTML() const
 
 DOMString HTMLElementImpl::outerHTML() const
 {
-    return recursive_toHTML(true);
+    return recursive_toHTML();
 }
 
 DOMString HTMLElementImpl::innerText() const
index 4248777..27f5828 100644 (file)
@@ -850,7 +850,7 @@ DOMString RangeImpl::toHTML(QPtrList<NodeImpl> *nodes)
     }
     NodeImpl::Id id = commonAncestor->id();
     bool onlyIncludeChildren = (id != ID_TABLE && id != ID_OL && id != ID_UL);
-    return commonAncestor->recursive_toHTML(onlyIncludeChildren, this, nodes);
+    return commonAncestor->recursive_toHTML(onlyIncludeChildren, true, this, nodes);
 }
 
 DOMString RangeImpl::text() const
index dd3452b..4d15094 100644 (file)
@@ -254,11 +254,7 @@ NodeImpl *NodeImpl::addChild(NodeImpl *)
 
 QString NodeImpl::toHTML() const
 {
-    NodeImpl* fc = firstChild();
-    if ( fc )
-        return fc->recursive_toHTML(true);
-
-    return "";
+    return recursive_toHTML(true);
 }
 
 static QString escapeHTML( const QString& in )
@@ -283,12 +279,12 @@ static QString escapeHTML( const QString& in )
     return s;
 }
 
-QString NodeImpl::recursive_toString(const NodeImpl *startNode, bool onlyIncludeChildren, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes)
+QString NodeImpl::recursive_toString(const NodeImpl *startNode, bool onlyIncludeChildren, bool includeSiblings, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes)
 
 {
     QString me = "";
 
-    for (const NodeImpl *current = startNode; current != NULL; current = current->nextSibling()) {
+    for (const NodeImpl *current = startNode; current != NULL; current = includeSiblings ? current->nextSibling() : NULL) {
         bool include = true;
         if (onlyIncludeChildren) {
             // Don't include the HTML of this node, but include the children in the next recursion.
@@ -348,7 +344,7 @@ QString NodeImpl::recursive_toString(const NodeImpl *startNode, bool onlyInclude
         if (!current->isHTMLElement() || endTag[current->id()] != FORBIDDEN) {
             // print children
             if (NodeImpl *n = current->firstChild()) {
-                me += recursive_toString(n, false, range, nodes);
+                me += recursive_toString(n, false, true, range, nodes);
             }
             // Print my ending tag
             if (include && current->nodeType() != Node::TEXT_NODE && current->nodeType() != Node::DOCUMENT_NODE) {
@@ -364,9 +360,9 @@ QString NodeImpl::recursive_toString(const NodeImpl *startNode, bool onlyInclude
 
 }
 
-QString NodeImpl::recursive_toHTML(bool onlyIncludeChildren, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes) const
+QString NodeImpl::recursive_toHTML(bool onlyIncludeChildren, bool includeSiblings, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes) const
 {      
-    return NodeImpl::recursive_toString(this, onlyIncludeChildren, range, nodes);
+    return NodeImpl::recursive_toString(this, onlyIncludeChildren, includeSiblings, range, nodes);
 }
 
 void NodeImpl::recursive_completeURLs(QString baseURL)
index b65f6c1..dbb5ab6 100644 (file)
@@ -260,8 +260,8 @@ public:
     
     virtual bool isInline() const;
     virtual QString toHTML() const;
-    QString recursive_toHTML(bool onlyIncludeChildren=false, const DOM::RangeImpl *range=NULL, QPtrList<NodeImpl> *nodes=NULL) const;
-    static QString recursive_toString(const NodeImpl *startNode, bool onlyIncludeChildren=false, const DOM::RangeImpl *range=NULL, QPtrList<NodeImpl> *nodes=NULL);
+    QString recursive_toHTML(bool onlyIncludeChildren=false, bool includeSiblings = false, const DOM::RangeImpl *range=NULL, QPtrList<NodeImpl> *nodes=NULL) const;
+    static QString recursive_toString(const NodeImpl *startNode, bool onlyIncludeChildren, bool includeSiblings, const DOM::RangeImpl *range, QPtrList<NodeImpl> *nodes);
     void recursive_completeURLs(QString baseURL);
     
     virtual bool isContentEditable() const;
index ede632a..d68a951 100644 (file)
@@ -508,7 +508,7 @@ static bool initializedKJS = FALSE;
     if (nodes) {
         nodeList = new QPtrList<NodeImpl>;
     }
-    NSString *markupString = [node _nodeImpl]->recursive_toHTML(false, NULL, nodeList).getNSString();
+    NSString *markupString = [node _nodeImpl]->recursive_toHTML(false, false, NULL, nodeList).getNSString();
     if (nodes) {
         *nodes = [self nodesFromList:nodeList];
         delete nodeList;