Changeset 202895 in webkit

Timestamp:

Jul 6, 2016 10:55:30 PM (8 years ago)

Author:

Chris Dumez

Message:

Document.title setter does not work for SVG documents
​https://bugs.webkit.org/show_bug.cgi?id=159503
<rdar://problem/27212313>

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Rebaseline W3C test now that all checks are passing.

• web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-09-expected.txt:

Source/WebCore:

Document.title setter should work for SVG documents:

• ​https://html.spec.whatwg.org/multipage/dom.html#document.title

This patch aligns our behavior with the specification
and with Firefox / Chrome.

No new tests, rebaselined existing test.

• dom/Document.cpp:

(WebCore::Document::setTitle):

• Reverse the if conditions for clarity.
• If the document element is an SVG svg element, create a SVGTitleElement and insert it as first child of the document element.
• Call SVGTitleElement::setText() instead of HTMLTitleElement::setText() at the end of the method if m_titleElement is a SVGTitleElement.

(WebCore::Document::updateTitleElement):

• If document element is an SVG svg element, use the first child of the document element that is a SVGTitleElement.

• svg/SVGTitleElement.cpp:

(WebCore::SVGTitleElement::setText):

• svg/SVGTitleElement.h:

Add SVGTitleElement::setText() method that does the same
thing as HTMLTitleElement::setText().

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-09-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (4 diffs)
• Source/WebCore/html/HTMLTitleElement.cpp (modified) (1 diff)
• Source/WebCore/svg/SVGTitleElement.cpp (modified) (3 diffs)
• Source/WebCore/svg/SVGTitleElement.h (modified) (1 diff)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2016-07-06  Chris Dumez  <cdumez@apple.com>
+
+        Document.title setter does not work for SVG documents
+        https://bugs.webkit.org/show_bug.cgi?id=159503
+        <rdar://problem/27212313>
+
+        Reviewed by Ryosuke Niwa.
+
+        Rebaseline W3C test now that all checks are passing.
+
+        * web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-09-expected.txt:
+
 2016-07-06  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-09-expected.txt

@@ -1 +1 @@
 
-FAIL No title element in SVG document assert_equals: expected "title" but got "x-child"
-FAIL Title element in SVG document assert_equals: expected "foobar" but got "foo"
-FAIL Title element not child of SVG root assert_equals: expected "" but got "foo"
-FAIL Title element not in SVG namespace assert_equals: expected "" but got "foo"
-FAIL Root element not named "svg" assert_equals: expected "" but got "foo"
+PASS No title element in SVG document 
+PASS Title element in SVG document 
+PASS Title element not child of SVG root 
+PASS Title element not in SVG namespace 
+PASS Root element not named "svg" 
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-07-06  Chris Dumez  <cdumez@apple.com>
+
+        Document.title setter does not work for SVG documents
+        https://bugs.webkit.org/show_bug.cgi?id=159503
+        <rdar://problem/27212313>
+
+        Reviewed by Ryosuke Niwa.
+
+        Document.title setter should work for SVG documents:
+        - https://html.spec.whatwg.org/multipage/dom.html#document.title
+
+        This patch aligns our behavior with the specification
+        and with Firefox / Chrome.
+
+        No new tests, rebaselined existing test.
+
+        * dom/Document.cpp:
+        (WebCore::Document::setTitle):
+        - Reverse the if conditions for clarity.
+        - If the document element is an SVG svg element, create a
+          SVGTitleElement and insert it as first child of the
+          document element.
+        - Call SVGTitleElement::setText() instead of
+          HTMLTitleElement::setText() at the end of the method if
+          m_titleElement is a SVGTitleElement.
+
+        (WebCore::Document::updateTitleElement):
+        - If document element is an SVG svg element, use the first
+          child of the document element that is a SVGTitleElement.
+
+        * svg/SVGTitleElement.cpp:
+        (WebCore::SVGTitleElement::setText):
+        * svg/SVGTitleElement.h:
+        Add SVGTitleElement::setText() method that does the same
+        thing as HTMLTitleElement::setText().
+
 2016-07-06  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -4 +4 @@
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
- * Copyright (C) 2004-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2004-2016 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
@@ -147 +147 @@
 #include "SVGElementFactory.h"
 #include "SVGNames.h"
+#include "SVGSVGElement.h"
 #include "SVGTitleElement.h"
 #include "SVGZoomEvent.h"
@@ -1621 +1622 @@
 void Document::setTitle(const String& title)
 {
-    if (!isHTMLDocument() && !isXHTMLDocument())
+    if (!m_titleElement) {
+        if (isHTMLDocument() || isXHTMLDocument()) {
+            auto* headElement = head();
+            if (!headElement)
+                return;
+            m_titleElement = HTMLTitleElement::create(HTMLNames::titleTag, *this);
+            headElement->appendChild(*m_titleElement);
+        } else if (isSVGDocument()) {
+            auto* element = documentElement();
+            if (!is<SVGSVGElement>(element))
+                return;
+            m_titleElement = SVGTitleElement::create(SVGNames::titleTag, *this);
+            element->insertBefore(*m_titleElement, element->firstChild());
+        }
+    } else if (!isHTMLDocument() && !isXHTMLDocument() && !isSVGDocument())
         m_titleElement = nullptr;
-    else if (!m_titleElement) {
-        auto* headElement = head();
-        if (!headElement)
-            return;
-        m_titleElement = createElement(titleTag, false);
-        headElement->appendChild(*m_titleElement, ASSERT_NO_EXCEPTION);
-    }
 
     // The DOM API has no method of specifying direction, so assume LTR.
@@ -1636 +1644 @@
     if (is<HTMLTitleElement>(m_titleElement.get()))
         downcast<HTMLTitleElement>(*m_titleElement).setText(title);
+    else if (is<SVGTitleElement>(m_titleElement.get()))
+        downcast<SVGTitleElement>(*m_titleElement).setText(title);
 }
 
 void Document::updateTitleElement(Element* newTitleElement)
 {
-    // Only allow the first title element in tree order to change the title -- others have no effect.
-    if (m_titleElement) {
-        if (isHTMLDocument() || isXHTMLDocument())
-            m_titleElement = descendantsOfType<HTMLTitleElement>(*this).first();
-        else if (isSVGDocument())
-            m_titleElement = descendantsOfType<SVGTitleElement>(*this).first();
-    } else
-        m_titleElement = newTitleElement;
+    if (is<SVGSVGElement>(documentElement()))
+        m_titleElement = childrenOfType<SVGTitleElement>(*documentElement()).first();
+    else {
+        if (m_titleElement) {
+            if (isHTMLDocument() || isXHTMLDocument())
+                m_titleElement = descendantsOfType<HTMLTitleElement>(*this).first();
+        } else
+            m_titleElement = newTitleElement;
+    }
 
     updateTitleFromTitleElement();

trunk/Source/WebCore/html/HTMLTitleElement.cpp

@@ -101 +101 @@
     // We make a copy here because entity of "value" argument can be Document::m_title,
     // which goes empty during removeChildren() invocation below,
-    // which causes HTMLTitleElement::childrenChanged(), which ends up Document::setTitle().
+    // which causes HTMLTitleElement::childrenChanged(), which ends up calling Document::setTitle().
     String valueCopy(value);
 

trunk/Source/WebCore/svg/SVGTitleElement.cpp

@@ -2 +2 @@
  * Copyright (C) 2004, 2005 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005, 2006 Rob Buis <buis@kde.org>
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ -24 +25 @@
 #include "Document.h"
 #include "SVGNames.h"
+#include "Text.h"
 
 namespace WebCore {
@@ -62 +64 @@
 }
 
+void SVGTitleElement::setText(const String& value)
+{
+    Ref<SVGTitleElement> protectedThis(*this);
+
+    if (!value.isEmpty() && hasOneChild() && is<Text>(*firstChild())) {
+        downcast<Text>(*firstChild()).setData(value);
+        return;
+    }
+
+    // We make a copy here because entity of "value" argument can be Document::m_title,
+    // which goes empty during removeChildren() invocation below,
+    // which causes SVGTitleElement::childrenChanged(), which ends up calling Document::setTitle().
+    String valueCopy(value);
+
+    if (hasChildNodes())
+        removeChildren();
+
+    if (!valueCopy.isEmpty())
+        appendChild(document().createTextNode(valueCopy), IGNORE_EXCEPTION);
 }
+
+}

trunk/Source/WebCore/svg/SVGTitleElement.h

@@ -31 +31 @@
     static Ref<SVGTitleElement> create(const QualifiedName&, Document&);
 
+    void setText(const String&);
+
 private:
     SVGTitleElement(const QualifiedName&, Document&);