[webkit-reviews] review denied: [Bug 23940] Use Document::createElement(const QualifiedName&, bool) when creating a known element inside WebCore : [Attachment 27635] Proposed changes

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 23940: Use Document::createElement(const QualifiedName&, bool) when
creating a known element inside WebCore
https://bugs.webkit.org/show_bug.cgi?id=23940

Attachment 27635: Proposed changes
https://bugs.webkit.org/attachment.cgi?id=27635&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
@@ -969,9 +969,8 @@ void Document::setTitle(const String& ti
	     m_titleElement = 0;
	 else if (!m_titleElement) {
	     if (HTMLElement* headElement = head()) {
+		 m_titleElement = createElement(titleTag, false);
		 ExceptionCode ec = 0;
-		 m_titleElement = createElement("title", ec);

I think that this changes the behavior for SVG documents - previously,
QualifiedName(nullAtom, "title", nullAtom) was used via createElement(). Is
this intentional? Please add a test case for this regardless.

-    RefPtr<Element> reportElement =
doc->createElementNS(HTMLNames::xhtmlNamespaceURI, "parsererror", ec);
-    reportElement->setAttribute(HTMLNames::styleAttr, "display: block;
white-space: pre; border: 2px solid #c77; padding: 0 1em 0 1em; margin: 1em;
background-color: #fdd; color: black");
+    RefPtr<Element> reportElement = doc->createElement(QualifiedName(nullAtom,
"parsererror", xhtmlNamespaceURI), true);

This changes createdByParser from false to true - is it intentional? I don't
see how that is correct.

Same question elsewhere in error header creation code.

-    // make the span to hold the tab
+    // Make the span to hold the tab

It's great to fix comments - we also prefer to have periods at the end of full
sentences.

I like this patch a lot! r- for now, to consider the comments.