Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(199)

Unified Diff: third_party/WebKit/Source/core/dom/Document.cpp

Issue 1752713002: Fix SVGDocument return title that is not a child of the root element (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Add test Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/dom/Document.cpp
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
index 64f72e837e40d3a5cb3d7d72c4b4a0cc95adb60d..b7e88253f27fd8620bd20814865325446f0d4627 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -1286,8 +1286,13 @@ void Document::updateTitle(const String& title)
void Document::setTitle(const String& title)
{
// Title set by JavaScript -- overrides any title elements.
- if (!isHTMLDocument() && !isXHTMLDocument()) {
+ if (!isHTMLDocument() && !isXHTMLDocument() && !isSVGDocument()) {
fs 2016/03/01 15:49:03 It would probably make sense to just reverse these
hyunjunekim2 2016/03/03 14:06:10 Done.
m_titleElement = nullptr;
+ } else if (isSVGDocument()) {
+ if (!m_titleElement) {
+ m_titleElement = SVGTitleElement::create(*this);
+ documentElement()->insertBefore(m_titleElement.get(), documentElement()->firstChild());
fs 2016/03/01 15:49:03 Check that documentElement() exists.
hyunjunekim2 2016/03/03 14:06:10 Done.
+ }
} else if (!m_titleElement) {
HTMLElement* headElement = head();
if (!headElement)
@@ -1296,10 +1301,13 @@ void Document::setTitle(const String& title)
headElement->appendChild(m_titleElement.get());
}
- if (isHTMLTitleElement(m_titleElement))
+ if (isHTMLTitleElement(m_titleElement)) {
toHTMLTitleElement(m_titleElement)->setText(title);
- else
+ } else if (isSVGTitleElement(m_titleElement)) {
+ toSVGTitleElement(m_titleElement)->setTextContent(title);
fs 2016/03/01 15:49:03 Don't we need protection against mutation here? (L
hyunjunekim2 2016/03/03 14:06:10 Done. made SVGTitleElement::setText that referred
+ } else {
updateTitle(title);
+ }
}
void Document::setTitleElement(Element* titleElement)
@@ -1315,10 +1323,24 @@ void Document::setTitleElement(Element* titleElement)
m_titleElement = titleElement;
}
- if (isHTMLTitleElement(m_titleElement))
+ if (isHTMLTitleElement(m_titleElement)) {
updateTitle(toHTMLTitleElement(m_titleElement)->text());
- else if (isSVGTitleElement(m_titleElement))
- updateTitle(toSVGTitleElement(m_titleElement)->textContent());
+ } else if (isSVGTitleElement(m_titleElement)) {
+ SVGElement* rootElement = toSVGElement(documentElement());
fs 2016/03/01 15:49:03 Dangerous (but unnecessary)
hyunjunekim2 2016/03/03 14:06:10 Done. Removed it.
+ // If the root element is an svg element, the first title element in the SVG namespace
fs 2016/03/01 15:49:03 It looks like a better fit to do this above?
+ // is a child of the root element.
+ bool hasSVGTitle = false;
+ for (SVGElement* child = Traversal<SVGElement>::firstChild(*rootElement); child; child = Traversal<SVGElement>::nextSibling(*child)) {
fs 2016/03/01 15:49:03 This appears to be a very long-winded way of check
hyunjunekim2 2016/03/03 14:06:10 Done.
+ if (child == m_titleElement) {
+ updateTitle(toSVGTitleElement(m_titleElement)->textContent());
fs 2016/03/01 15:49:03 Spec says: "If the root element is an svg element
hyunjunekim2 2016/03/03 14:06:10 Done. passed document.title-09.html.
+ hasSVGTitle = true;
+ break;
+ }
+ }
+
+ if (!hasSVGTitle)
+ m_titleElement = nullptr;
+ }
}
void Document::removeTitle(Element* titleElement)

Powered by Google App Engine
This is Rietveld 408576698