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

Side by Side 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, 9 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 unified diff | Download patch
OLDNEW
1 /* 1 /*
2 * Copyright (C) 1999 Lars Knoll (knoll@kde.org) 2 * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
3 * (C) 1999 Antti Koivisto (koivisto@kde.org) 3 * (C) 1999 Antti Koivisto (koivisto@kde.org)
4 * (C) 2001 Dirk Mueller (mueller@kde.org) 4 * (C) 2001 Dirk Mueller (mueller@kde.org)
5 * (C) 2006 Alexey Proskuryakov (ap@webkit.org) 5 * (C) 2006 Alexey Proskuryakov (ap@webkit.org)
6 * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All r ights reserved. 6 * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All r ights reserved.
7 * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.t orchmobile.com/) 7 * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.t orchmobile.com/)
8 * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved. 8 * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
9 * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies) 9 * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
10 * Copyright (C) Research In Motion Limited 2010-2011. All rights reserved. 10 * Copyright (C) Research In Motion Limited 2010-2011. All rights reserved.
(...skipping 1268 matching lines...) Expand 10 before | Expand all | Expand 10 after
1279 m_title = canonicalizedTitle<UChar>(this, m_rawTitle); 1279 m_title = canonicalizedTitle<UChar>(this, m_rawTitle);
1280 1280
1281 if (!m_frame || oldTitle == m_title) 1281 if (!m_frame || oldTitle == m_title)
1282 return; 1282 return;
1283 m_frame->loader().client()->dispatchDidReceiveTitle(m_title); 1283 m_frame->loader().client()->dispatchDidReceiveTitle(m_title);
1284 } 1284 }
1285 1285
1286 void Document::setTitle(const String& title) 1286 void Document::setTitle(const String& title)
1287 { 1287 {
1288 // Title set by JavaScript -- overrides any title elements. 1288 // Title set by JavaScript -- overrides any title elements.
1289 if (!isHTMLDocument() && !isXHTMLDocument()) { 1289 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.
1290 m_titleElement = nullptr; 1290 m_titleElement = nullptr;
1291 } else if (isSVGDocument()) {
1292 if (!m_titleElement) {
1293 m_titleElement = SVGTitleElement::create(*this);
1294 documentElement()->insertBefore(m_titleElement.get(), documentElemen t()->firstChild());
fs 2016/03/01 15:49:03 Check that documentElement() exists.
hyunjunekim2 2016/03/03 14:06:10 Done.
1295 }
1291 } else if (!m_titleElement) { 1296 } else if (!m_titleElement) {
1292 HTMLElement* headElement = head(); 1297 HTMLElement* headElement = head();
1293 if (!headElement) 1298 if (!headElement)
1294 return; 1299 return;
1295 m_titleElement = HTMLTitleElement::create(*this); 1300 m_titleElement = HTMLTitleElement::create(*this);
1296 headElement->appendChild(m_titleElement.get()); 1301 headElement->appendChild(m_titleElement.get());
1297 } 1302 }
1298 1303
1299 if (isHTMLTitleElement(m_titleElement)) 1304 if (isHTMLTitleElement(m_titleElement)) {
1300 toHTMLTitleElement(m_titleElement)->setText(title); 1305 toHTMLTitleElement(m_titleElement)->setText(title);
1301 else 1306 } else if (isSVGTitleElement(m_titleElement)) {
1307 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
1308 } else {
1302 updateTitle(title); 1309 updateTitle(title);
1310 }
1303 } 1311 }
1304 1312
1305 void Document::setTitleElement(Element* titleElement) 1313 void Document::setTitleElement(Element* titleElement)
1306 { 1314 {
1307 // Only allow the first title element to change the title -- others have no effect. 1315 // Only allow the first title element to change the title -- others have no effect.
1308 if (m_titleElement && m_titleElement != titleElement) { 1316 if (m_titleElement && m_titleElement != titleElement) {
1309 if (isHTMLDocument() || isXHTMLDocument()) { 1317 if (isHTMLDocument() || isXHTMLDocument()) {
1310 m_titleElement = Traversal<HTMLTitleElement>::firstWithin(*this); 1318 m_titleElement = Traversal<HTMLTitleElement>::firstWithin(*this);
1311 } else if (isSVGDocument()) { 1319 } else if (isSVGDocument()) {
1312 m_titleElement = Traversal<SVGTitleElement>::firstWithin(*this); 1320 m_titleElement = Traversal<SVGTitleElement>::firstWithin(*this);
1313 } 1321 }
1314 } else { 1322 } else {
1315 m_titleElement = titleElement; 1323 m_titleElement = titleElement;
1316 } 1324 }
1317 1325
1318 if (isHTMLTitleElement(m_titleElement)) 1326 if (isHTMLTitleElement(m_titleElement)) {
1319 updateTitle(toHTMLTitleElement(m_titleElement)->text()); 1327 updateTitle(toHTMLTitleElement(m_titleElement)->text());
1320 else if (isSVGTitleElement(m_titleElement)) 1328 } else if (isSVGTitleElement(m_titleElement)) {
1321 updateTitle(toSVGTitleElement(m_titleElement)->textContent()); 1329 SVGElement* rootElement = toSVGElement(documentElement());
fs 2016/03/01 15:49:03 Dangerous (but unnecessary)
hyunjunekim2 2016/03/03 14:06:10 Done. Removed it.
1330 // 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?
1331 // is a child of the root element.
1332 bool hasSVGTitle = false;
1333 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.
1334 if (child == m_titleElement) {
1335 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.
1336 hasSVGTitle = true;
1337 break;
1338 }
1339 }
1340
1341 if (!hasSVGTitle)
1342 m_titleElement = nullptr;
1343 }
1322 } 1344 }
1323 1345
1324 void Document::removeTitle(Element* titleElement) 1346 void Document::removeTitle(Element* titleElement)
1325 { 1347 {
1326 if (m_titleElement != titleElement) 1348 if (m_titleElement != titleElement)
1327 return; 1349 return;
1328 1350
1329 m_titleElement = nullptr; 1351 m_titleElement = nullptr;
1330 1352
1331 // Update title based on first title element in the document, if one exists. 1353 // Update title based on first title element in the document, if one exists.
(...skipping 4663 matching lines...) Expand 10 before | Expand all | Expand 10 after
5995 #ifndef NDEBUG 6017 #ifndef NDEBUG
5996 using namespace blink; 6018 using namespace blink;
5997 void showLiveDocumentInstances() 6019 void showLiveDocumentInstances()
5998 { 6020 {
5999 Document::WeakDocumentSet& set = Document::liveDocumentSet(); 6021 Document::WeakDocumentSet& set = Document::liveDocumentSet();
6000 fprintf(stderr, "There are %u documents currently alive:\n", set.size()); 6022 fprintf(stderr, "There are %u documents currently alive:\n", set.size());
6001 for (Document* document : set) 6023 for (Document* document : set)
6002 fprintf(stderr, "- Document %p URL: %s\n", document, document->url().str ing().utf8().data()); 6024 fprintf(stderr, "- Document %p URL: %s\n", document, document->url().str ing().utf8().data());
6003 } 6025 }
6004 #endif 6026 #endif
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698