[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
bugzilla-daemon at webkit.org
Tue Aug 7 23:01:05 PDT 2012
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.
More information about the webkit-reviews
mailing list