Document.title getter should return text of title element and setter should stop when head el…

Source/core/dom/Document.cpp

Index: Source/core/dom/Document.cpp
diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
index 6d57dec3e71fe4aaf548661c663bab818b53eaee..f3d76cff1ed15db15f3b244b087c09c46652cab7 100644
--- a/Source/core/dom/Document.cpp
+++ b/Source/core/dom/Document.cpp
@@ -459,7 +459,6 @@ Document::Document(const DocumentInit& initializer, DocumentClassFlags documentC
     , m_updateFocusAppearanceRestoresSelection(false)
     , m_containsPlugins(false)
     , m_ignoreDestructiveWriteCount(0)
-    , m_titleSetExplicitly(false)
     , m_markers(adoptPtrWillBeNoop(new DocumentMarkerController))
     , m_updateFocusAppearanceTimer(this, &Document::updateFocusAppearanceTimerFired)
     , m_cssTarget(nullptr)
@@ -1387,14 +1386,14 @@ void Document::updateTitle(const String& title)
 void Document::setTitle(const String& title)
 {
     // Title set by JavaScript -- overrides any title elements.
-    m_titleSetExplicitly = true;
     if (!isHTMLDocument() && !isXHTMLDocument())
         m_titleElement = nullptr;
     else if (!m_titleElement) {
-        if (HTMLElement* headElement = head()) {
-            m_titleElement = HTMLTitleElement::create(*this);
-            headElement->appendChild(m_titleElement.get());
-        }
+        HTMLElement* headElement = head();
+        if (!isSVGDocument() && !headElement)

[Inactive, 2014/07/16 14:42:05]

This !isSVGDocument() check seems superfluous. If it were an SVGDocument, it
would execute the if case, not the else if one I believe.

[kangil_, 2014/07/18 06:55:58]

Done.

+            return;
+        m_titleElement = HTMLTitleElement::create(*this);
+        headElement->appendChild(m_titleElement.get());
     }
 
     if (isHTMLTitleElement(m_titleElement))
@@ -1405,14 +1404,14 @@ void Document::setTitle(const String& title)
 
 void Document::setTitleElement(const String& title, Element* titleElement)

[Inactive, 2014/07/16 14:42:05]

It looks like you broke the SVG case in this method. You now only handle
HTMLTitleElement properly. How about SVGTitleElement? I think this is why your
svg test cases are failing.

[kangil_, 2014/07/18 06:55:58]

Done.

 {
-    if (titleElement != m_titleElement) {
-        if (m_titleElement || m_titleSetExplicitly)
-            // Only allow the first title element to change the title -- others have no effect.
-            return;
+    // Only allow the first title element to change the title -- others have no effect.
+    if (m_titleElement && m_titleElement != titleElement)
+        m_titleElement = Traversal<HTMLTitleElement>::next(*this);

[Inactive, 2014/07/16 14:42:05]

Wouldn't Traversal<HTMLTitleElement>::firstWithin(*this) be clearer?

[kangil_, 2014/07/18 06:55:58]

Done.

+    else
         m_titleElement = titleElement;
-    }
 
-    updateTitle(title);
+    if (isHTMLTitleElement(m_titleElement))
+        updateTitle(toHTMLTitleElement(m_titleElement)->text());
 }
 
 void Document::removeTitle(Element* titleElement)
@@ -1421,14 +1420,11 @@ void Document::removeTitle(Element* titleElement)
         return;
 
     m_titleElement = nullptr;
-    m_titleSetExplicitly = false;
 
     // FIXME: This is broken for SVG.
-    // Update title based on first title element in the head, if one exists.
-    if (HTMLElement* headElement = head()) {
-        if (HTMLTitleElement* title = Traversal<HTMLTitleElement>::firstChild(*headElement))
-            setTitleElement(title->text(), title);
-    }
+    // Update title based on first title element in the document, if one exists.
+    if (HTMLTitleElement* title = Traversal<HTMLTitleElement>::next(*this))

[Inactive, 2014/07/16 14:42:05]

firstWithin?

[kangil_, 2014/07/18 06:55:58]

Done.

+        setTitleElement(title->text(), title);
 
     if (!m_titleElement)
         updateTitle(String());