355451 - Be more efficient about creating HTML element

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

NS_NewHTMLElement acts case insensivly to the local name of the nodeinfo passed in when creating HTML elements. So that means that if you create a nodeinfo with an uppercase name 'INPUT' we'll still create an nsHTMLInputElement, as well as give it a nodeinfo with a lowercase localname.

However it's much more efficient to lowercase the name before creating the nodeinfo. First of all callers run the risk of looking up the localname atom and the nodeinfo in vain since we'll just dicard them inside NS_NewHTMLElement. 

Also, it create inconsistent behaviour since only 'supported' element names are lowercased, if you try to create a 'FOOBAR' html element you'll get one with an uppercase localname atom.

Additionally all callers lowercase the name for known elements anyway. The only caller the doesn't lowercase names is nsHTMLFragmentContentSink, which doesn't lowercase names for unknown elements.

Patch to change this so that all callers lowercase and making NS_NewHTMLElement not lowercase comming up. I also removed nsDocument::CreateElement since NS_NewElement does the exact same thing (although with arguments in reverse order).

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 241253 [details] [diff] [review]
Patch to fix

r+sr=jst

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in.

Comment 4

[[e-mail]]

Comment on attachment 241253 [details] [diff] [review]
Patch to fix

>+  NS_ASSERTION(aNodeType <= NS_HTML_TAG_MAX, "aNodeType is out of bounds");
So what does it mean if I hit this assertion? Minimal testcase is <o:p></o:p>

Comment 5

[Peter Van der Beken [:peterv]]

(In reply to comment #4)
> (From update of attachment 241253 [details] [diff] [review] [edit])
> >+  NS_ASSERTION(aNodeType <= NS_HTML_TAG_MAX, "aNodeType is out of bounds");
> So what does it mean if I hit this assertion? Minimal testcase is <o:p></o:p>

That the assertion is wrong, since eHTMLTag_userdefined > NS_HTML_TAG_MAX.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in a fix for the assertion.

Comment 7

[[e-mail]]

Comment on attachment 241253 [details] [diff] [review]
Patch to fix

>-  rv = CreateElement(nodeInfo, nodeInfo->NamespaceID(),
>-                     getter_AddRefs(content));
>+  NS_NewElement(getter_AddRefs(content), nodeInfo->NamespaceID(), nodeInfo);
>   NS_ENSURE_SUCCESS(rv, rv);
Oops.