[Webkit-unassigned] [Bug 23940] Use Document::createElement(const QualifiedName&, bool) when creating a known element inside WebCore

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=23940





------- Comment #3 from jchaffraix at webkit.org  2009-02-17 11:21 PDT -------
(In reply to comment #2)
> (From update of attachment 27635 [review])
> @@ -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.

Yes, it is. We are creating a title tag for an HTML document (there is a
"if(!HTMLDocument()) ... else" guard above). I can add a test case if you
request it in light of my point.

As you were pointing out, there will be a change in behaviour for all my
createElement(const AtomicString&, ...) -> createElement(const QualifiedName&,
bool) changes. I have double-checked all the changes and the only problematic
change I see is in DragController - the other changes are (X)HTML-specific. I
do not think that creating an element in the null namespace is right either.

> -    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.
> 

This is called from the parser so I have though it was correct to set
createdByParser to true. I have no strong feelings about the value given
thought.

> -    // 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.

Right, I have missed that.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.