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

Source/core/dom/Document.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 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.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) Research In Motion Limited 2010-2011. All rights reserved.
@@ skipping 441 matching lines @@
     , m_mutationObserverTypes(0)
     , m_visitedLinkState(VisitedLinkState::create(*this))
     , m_visuallyOrdered(false)
     , m_readyState(Complete)
     , m_isParsing(false)
     , m_gotoAnchorNeededAfterStylesheetsLoad(false)
     , m_containsValidityStyleRules(false)
     , 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)
     , m_loadEventProgress(LoadEventNotRun)
     , m_startTime(currentTime())
     , m_scriptRunner(ScriptRunner::create(this))
     , m_xmlVersion("1.0")
     , m_xmlStandalone(StandaloneUnspecified)
     , m_hasXMLDeclaration(0)
     , m_designMode(inherit)
@@ skipping 907 matching lines @@
         m_title = canonicalizedTitle<UChar>(this, m_rawTitle);
 
     if (!m_frame || oldTitle == m_title)
         return;
     m_frame->loader().client()->dispatchDidReceiveTitle(m_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))
         toHTMLTitleElement(m_titleElement)->setText(title);
     else
         updateTitle(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)
 {
     if (m_titleElement != 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());
 }
 
 const AtomicString& Document::dir()
 {
     Element* rootElement = documentElement();
     if (isHTMLHtmlElement(rootElement))
         return toHTMLHtmlElement(rootElement)->dir();
@@ skipping 4409 matching lines @@
 using namespace WebCore;
 void showLiveDocumentInstances()
 {
     WeakDocumentSet& set = liveDocumentSet();
     fprintf(stderr, "There are %u documents currently alive:\n", set.size());
     for (WeakDocumentSet::const_iterator it = set.begin(); it != set.end(); ++it) {
         fprintf(stderr, "- Document %p URL: %s\n", *it, (*it)->url().string().utf8().data());
     }
 }
 #endif