[webkit-reviews] review denied: [Bug 86363] Add the event handler content attributes that are defined in the spec to HTMLElement : [Attachment 154273] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Keishi Hattori <keishi at webkit.org>'s
request for review:
Bug 86363: Add the event handler content attributes that are defined in the
spec to HTMLElement
https://bugs.webkit.org/show_bug.cgi?id=86363

Attachment 154273: Patch
https://bugs.webkit.org/attachment.cgi?id=154273&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=154273&action=review


> Source/WebCore/html/HTMLElement.cpp:220
> +    static HashMap<AtomicStringImpl*, AtomicString>*
attributeNameToEventNameMap = 0;

We don't have to use AtomicStringImpl any more. AtomicString's hash traits will
automatically take care of it for you.

> Source/WebCore/html/HTMLElement.cpp:222
> +    if (!attributeNameToEventNameMap) {
> +	   attributeNameToEventNameMap = new HashMap<AtomicStringImpl*,
AtomicString>;

Instead of dynamically allocating the hash map, you could have just checked the
size.
Also, if we're dynamically allocating this object, then we must use OwnPtr. r-
because of this.

> LayoutTests/fast/events/event-attribute.html:27
> +    var el = document.createElement(elementTags[i]);

Please don't use abbreviations like el.