Reviewed by John.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Apr 2005 15:55:02 +0000 (15:55 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Apr 2005 15:55:02 +0000 (15:55 +0000)
        - fixed <rdar://problem/4097849> REGRESSION (162-163): importNode creates non-HTML elements, thus style attributes (and some others) don't work

        * khtml/xml/dom_docimpl.h: Add virtual functions HTMLElementNamespace and isHTMLNamespace.
        * khtml/xml/dom_docimpl.cpp:
        (DocumentImpl::importNode): Rearranged this function and fixed the following problems: 1) made sure to ref node while attributes are
        being set on it so it doesn't get destroyed; 2) fixed code to get namespace from the element we are importing to use the IDs from
        the source document, not the destination document; 3) removed unneeded getDocument() call which just returns this; 4) fixed error
        handling for cases where an exception happens while processing the children.
        (DocumentImpl::HTMLElementNamespace): Added. Returns XHTML_NAMESPACE.
        (DocumentImpl::isHTMLNamespace): Added. Returns true for any namespace that matches XHTML_NAMESPACE (case insensitive).
        (DocumentImpl::createElementNS): Changed to call isHTMLNamespace, which will cause it to accept the null namespace in an HTML document.
        This is the change that fixes the bug. Also fixed the code path to do a little less wasteful work in the non-XHTML case.
        (DocumentImpl::createHTMLElement): Pass in HTMLElementNamespace() rather than 0 to tagId.
        (DocumentImpl::attrId): Use isHTMLNamespace instead of allowing the null namespace explicitly.
        (DocumentImpl::tagId): Ditto.

        * khtml/html/html_documentimpl.h: Add overrides for HTMLElementNamespace and isHTMLNamespace.
        * khtml/html/html_documentimpl.cpp:
        (HTMLDocumentImpl::HTMLElementNamespace): Added. Returns 0 so we use the null string for HTML elements inside HTML documents (as before).
        (HTMLDocumentImpl::isHTMLNamespace): Added. Allows 0, and then calls base class to check for the actual XHTML namespace. Thus, we allow
        both no namespace at all and the XHTML namespace inside HTML documents.

        * khtml/html/html_elementimpl.cpp: (HTMLElementImpl::namespaceURI): Changed to call HTMLElementNamespace rather than checking
        isHTMLDocument. Same result as before, but better division of responsibilities.

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/html/html_documentimpl.cpp
WebCore/khtml/html/html_documentimpl.h
WebCore/khtml/html/html_elementimpl.cpp
WebCore/khtml/xml/dom_docimpl.cpp
WebCore/khtml/xml/dom_docimpl.h

index ddad3f49588fac07f69d160fe18e8b1b7ca8f016..93e8631971be004adaa0ee8af38bffacf5275ec2 100644 (file)
@@ -1,3 +1,32 @@
+2005-04-25  Darin Adler  <darin@apple.com>
+
+        Reviewed by John.
+
+        - fixed <rdar://problem/4097849> REGRESSION (162-163): importNode creates non-HTML elements, thus style attributes (and some others) don't work
+
+        * khtml/xml/dom_docimpl.h: Add virtual functions HTMLElementNamespace and isHTMLNamespace.
+        * khtml/xml/dom_docimpl.cpp:
+        (DocumentImpl::importNode): Rearranged this function and fixed the following problems: 1) made sure to ref node while attributes are
+        being set on it so it doesn't get destroyed; 2) fixed code to get namespace from the element we are importing to use the IDs from
+        the source document, not the destination document; 3) removed unneeded getDocument() call which just returns this; 4) fixed error
+        handling for cases where an exception happens while processing the children.
+        (DocumentImpl::HTMLElementNamespace): Added. Returns XHTML_NAMESPACE.
+        (DocumentImpl::isHTMLNamespace): Added. Returns true for any namespace that matches XHTML_NAMESPACE (case insensitive).
+        (DocumentImpl::createElementNS): Changed to call isHTMLNamespace, which will cause it to accept the null namespace in an HTML document.
+        This is the change that fixes the bug. Also fixed the code path to do a little less wasteful work in the non-XHTML case.
+        (DocumentImpl::createHTMLElement): Pass in HTMLElementNamespace() rather than 0 to tagId.
+        (DocumentImpl::attrId): Use isHTMLNamespace instead of allowing the null namespace explicitly.
+        (DocumentImpl::tagId): Ditto.
+
+        * khtml/html/html_documentimpl.h: Add overrides for HTMLElementNamespace and isHTMLNamespace.
+        * khtml/html/html_documentimpl.cpp:
+        (HTMLDocumentImpl::HTMLElementNamespace): Added. Returns 0 so we use the null string for HTML elements inside HTML documents (as before).
+        (HTMLDocumentImpl::isHTMLNamespace): Added. Allows 0, and then calls base class to check for the actual XHTML namespace. Thus, we allow
+        both no namespace at all and the XHTML namespace inside HTML documents.
+
+        * khtml/html/html_elementimpl.cpp: (HTMLElementImpl::namespaceURI): Changed to call HTMLElementNamespace rather than checking
+        isHTMLDocument. Same result as before, but better division of responsibilities.
+
 2005-04-25  Darin Adler  <darin@apple.com>
 
         Reviewed by John.
index e184dd5c8ac489d6c20e026856dab3db9544b6ca..d23b2f8cc0c81640fe21a776836dd040a7c15b98 100644 (file)
@@ -533,4 +533,16 @@ void HTMLDocumentImpl::determineParseMode( const QString &str )
  
 }
 
+DOMStringImpl *HTMLDocumentImpl::HTMLElementNamespace() const
+{
+    // For HTML documents, we treat HTML elements as having no namespace.
+    return 0;
+}
+
+bool HTMLDocumentImpl::isHTMLNamespace(DOMStringImpl *n) const
+{
+    // For HTML documents, either "no namespace" or the HTML namespace qualifies.
+    return !n || DocumentImpl::isHTMLNamespace(n);
+}
+
 #include "html_documentimpl.moc"
index 06d9c8261f47fa2039203259a11587db7485c885..22c1f226ff877c53f77be4cd237b404ee9109dcb 100644 (file)
@@ -67,6 +67,9 @@ public:
 
     HTMLCollectionImpl::CollectionInfo *collectionInfo(int type) { return m_collection_info+type; }
 
+    virtual DOMStringImpl *HTMLElementNamespace() const;
+    virtual bool isHTMLNamespace(DOMStringImpl *) const;
+
 protected:
     HTMLElementImpl *bodyElement;
     HTMLElementImpl *htmlElement;
index 36941e1fa083773d60474b26dba794b3f1a2a437..b3bd86c096c979a6c826ae93bd30b086644413ea 100644 (file)
@@ -961,15 +961,9 @@ bool HTMLElementImpl::setOuterText( const DOMString &text )
     return true;
 }
 
-
 DOMString HTMLElementImpl::namespaceURI() const
 {
-    // For HTML documents, we treat HTML elements as having no namespace. But for XML documents
-    // the elements have the namespace defined in the XHTML spec
-    if (getDocument()->isHTMLDocument())
-        return DOMString();
-    else
-        return XHTML_NAMESPACE;
+    return getDocument()->HTMLElementNamespace();
 }
 
 void HTMLElementImpl::addHTMLAlignment(HTMLAttributeImpl* attr)
index ac7349ebe8df5ed56e8994491ba54bebcb8e4171..fc2223ce79ba83a665503270f91f8b185d0ff82c 100644 (file)
@@ -531,83 +531,99 @@ CSSStyleDeclarationImpl *DocumentImpl::createCSSStyleDeclaration()
 
 NodeImpl *DocumentImpl::importNode(NodeImpl *importedNode, bool deep, int &exceptioncode)
 {
-       NodeImpl *result = 0;
-
-       if(importedNode->nodeType() == Node::ELEMENT_NODE)
-       {
-               ElementImpl *tempElementImpl = createElementNS(getDocument()->namespaceURI(id()), importedNode->nodeName(), exceptioncode);
-                if (exceptioncode)
-                    return 0;
-               result = tempElementImpl;
-
-               if(static_cast<ElementImpl *>(importedNode)->attributes(true) && static_cast<ElementImpl *>(importedNode)->attributes(true)->length())
-               {
-                       NamedNodeMapImpl *attr = static_cast<ElementImpl *>(importedNode)->attributes();
-
-                       for(unsigned int i = 0; i < attr->length(); i++)
-                       {
-                               DOMString qualifiedName = attr->item(i)->nodeName();
-                               DOMString value = attr->item(i)->nodeValue();
-
-                               int colonpos = qualifiedName.find(':');
-                               DOMString localName = qualifiedName;
-                               if(colonpos >= 0)
-                               {
-                                       localName.remove(0, colonpos + 1);
-                                       // ### extract and set new prefix
-                               }
-
-                               NodeImpl::Id nodeId = getDocument()->attrId(getDocument()->namespaceURI(id()), localName.implementation(), false /* allocate */);
-                               tempElementImpl->setAttribute(nodeId, value.implementation(), exceptioncode);
-
-                               if(exceptioncode != 0)
-                                       break;
-                       }
-               }
-       }
-       else if(importedNode->nodeType() == Node::TEXT_NODE)
-       {
-               result = createTextNode(importedNode->nodeValue());
-               deep = false;
-       }
-       else if(importedNode->nodeType() == Node::CDATA_SECTION_NODE)
-       {
-               result = createCDATASection(importedNode->nodeValue());
-               deep = false;
-       }
-       else if(importedNode->nodeType() == Node::ENTITY_REFERENCE_NODE)
-               result = createEntityReference(importedNode->nodeName());
-       else if(importedNode->nodeType() == Node::PROCESSING_INSTRUCTION_NODE)
-       {
-               result = createProcessingInstruction(importedNode->nodeName(), importedNode->nodeValue());
-               deep = false;
-       }
-       else if(importedNode->nodeType() == Node::COMMENT_NODE)
-       {
-               result = createComment(importedNode->nodeValue());
-               deep = false;
-       }
-       else
-               exceptioncode = DOMException::NOT_SUPPORTED_ERR;
+    exceptioncode = 0;
 
-       if(deep)
-       {
-               for(Node n = importedNode->firstChild(); !n.isNull(); n = n.nextSibling())
-                       result->appendChild(importNode(n.handle(), true, exceptioncode), exceptioncode);
-       }
+    switch (importedNode->nodeType()) {
+        case Node::TEXT_NODE:
+            return createTextNode(importedNode->nodeValue());
+        case Node::CDATA_SECTION_NODE:
+            return createCDATASection(importedNode->nodeValue());
+        case Node::ENTITY_REFERENCE_NODE:
+            return createEntityReference(importedNode->nodeName());
+        case Node::PROCESSING_INSTRUCTION_NODE:
+            return createProcessingInstruction(importedNode->nodeName(), importedNode->nodeValue());
+        case Node::COMMENT_NODE:
+            return createComment(importedNode->nodeValue());
+        case Node::ELEMENT_NODE: {
+            ElementImpl *e = createElementNS(importedNode->getDocument()->namespaceURI(importedNode->id()), importedNode->nodeName(), exceptioncode);
+            if (exceptioncode)
+                return 0;
+
+            e->ref();
+            if (static_cast<ElementImpl *>(importedNode)->attributes(true) && static_cast<ElementImpl *>(importedNode)->attributes(true)->length()) {
+                NamedAttrMapImpl *attrs = static_cast<ElementImpl *>(importedNode)->attributes();
+                unsigned length = attrs->length();
+                for (unsigned i = 0; i < length; i++) {
+                    AttrImpl *attr = attrs->item(i);
+                    DOMString qualifiedName = attr->nodeName();
+                    DOMString value = attr->nodeValue();
+
+                    int colonpos = qualifiedName.find(':');
+                    DOMString localName = qualifiedName;
+                    if (colonpos >= 0) {
+                        localName.remove(0, colonpos + 1);
+                        // ### extract and set new prefix
+                    }
+
+                    NodeImpl::Id nodeId = attrId(importedNode->getDocument()->namespaceURI(attr->attrImpl()->id()), localName.implementation(), false /* allocate */);
+                    e->setAttribute(nodeId, value.implementation(), exceptioncode);
+                    if (exceptioncode != 0) {
+                        e->deref();
+                        return 0;
+                    }
+                }
+            }
+
+            if (deep) {
+                for (NodeImpl *n = importedNode->firstChild(); n; n = n->nextSibling()) {
+                    NodeImpl *newNode = importNode(n, true, exceptioncode);
+                    if (exceptioncode != 0) {
+                        e->deref();
+                        return 0;
+                    }
+                    newNode->ref();
+                    e->appendChild(newNode, exceptioncode);
+                    if (exceptioncode != 0) {
+                        newNode->deref();
+                        e->deref();
+                        return 0;
+                    }
+                }
+            }
+
+            // Trick to get the result back to the floating state, with 0 refs but not destroyed.
+            e->setParent(this);
+            e->deref();
+            e->setParent(0);
+
+            return e;
+        }
+    }
+    exceptioncode = DOMException::NOT_SUPPORTED_ERR;
+    return 0;
+}
 
-       return result;
+DOMStringImpl *DocumentImpl::HTMLElementNamespace() const
+{
+    // For XML documents, HTML elements have the namespace defined in the XHTML specification.
+    static DOMString result = XHTML_NAMESPACE;
+    return result.implementation();
+}
+
+bool DocumentImpl::isHTMLNamespace(DOMStringImpl *n) const
+{
+    return strcasecmp(DOMString(n), XHTML_NAMESPACE) == 0;
 }
 
 ElementImpl *DocumentImpl::createElementNS( const DOMString &_namespaceURI, const DOMString &_qualifiedName, int &exceptioncode)
 {
     ElementImpl *e = 0;
-    QString qName = _qualifiedName.string();
-    int colonPos = qName.find(':',0);
 
-    if (_namespaceURI == XHTML_NAMESPACE) {
+    if (isHTMLNamespace(_namespaceURI.implementation())) {
         // User requested an element in the XHTML namespace - this means we create a HTML element
         // (elements not in this namespace are treated as normal XML elements)
+        QString qName = _qualifiedName.string();
+        int colonPos = qName.find(':',0);
         e = createHTMLElement(qName.mid(colonPos+1), exceptioncode);
         if (exceptioncode)
             return 0;
@@ -741,7 +757,7 @@ ElementImpl *DocumentImpl::createHTMLElement( const DOMString &name, int &except
         exceptioncode = DOMException::INVALID_CHARACTER_ERR;
         return 0;
     }
-    return createHTMLElement(tagId(0, name.implementation(), false));
+    return createHTMLElement(tagId(HTMLElementNamespace(), name.implementation(), false));
 }
 
 ElementImpl *DocumentImpl::createHTMLElement(unsigned short tagID)
@@ -2058,9 +2074,9 @@ NodeImpl::Id DocumentImpl::attrId(DOMStringImpl* _namespaceURI, DOMStringImpl *_
 
     // First see if it's a HTML attribute name
     QConstString n(_name->s, _name->l);
-    if (!_namespaceURI || !strcasecmp(_namespaceURI, XHTML_NAMESPACE)) {
+    if (isHTMLNamespace(_namespaceURI)) {
         // we're in HTML namespace if we know the tag.
-        // xhtml is lower case - case sensitive, easy to implement
+        // XHTML is lower case - case sensitive, easy to implement
         if ( htmlMode() == XHtml && (id = getAttrID(n.string().ascii(), _name->l)) )
             return id;
         // compatibility: upper case - case insensitive
@@ -2138,7 +2154,7 @@ NodeImpl::Id DocumentImpl::tagId(DOMStringImpl* _namespaceURI, DOMStringImpl *_n
 
     // First see if it's a HTML element name
     QConstString n(_name->s, _name->l);
-    if (!_namespaceURI || !strcasecmp(_namespaceURI, XHTML_NAMESPACE)) {
+    if (isHTMLNamespace(_namespaceURI)) {
         // we're in HTML namespace if we know the tag.
         // xhtml is lower case - case sensitive, easy to implement
         if ( htmlMode() == XHtml && (id = getTagID(n.string().ascii(), _name->l)) )
index bb4d63b7cf8fda944f87522ce40d699c58a0fd2b..6e2bb6678826cd93743b3e308fb181a15a88a741 100644 (file)
@@ -215,6 +215,9 @@ public:
     ElementImpl *createHTMLElement(const DOMString &tagName, int &exceptioncode);
     ElementImpl *createHTMLElement(unsigned short tagID);
 
+    virtual DOMStringImpl *HTMLElementNamespace() const;
+    virtual bool isHTMLNamespace(DOMStringImpl *) const;
+
     khtml::CSSStyleSelector *styleSelector() { return m_styleSelector; }
 
     ElementImpl *DocumentImpl::getElementByAccessKey( const DOMString &key );