[webkit-reviews] review denied: [Bug 26262] Implement HTML5 draggable : [Attachment 32253] Uses parseMappedAttribute instead of the user style sheet
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Jul 4 11:23:12 PDT 2009
Darin Adler <darin at apple.com> has denied Erik Arvidsson <arv at chromium.org>'s
request for review:
Bug 26262: Implement HTML5 draggable
https://bugs.webkit.org/show_bug.cgi?id=26262
Attachment 32253: Uses parseMappedAttribute instead of the user style sheet
https://bugs.webkit.org/attachment.cgi?id=32253&action=edit
------- Additional Comments from Darin Adler <darin at apple.com>
> + if (equalIgnoringCase(getAttribute(draggableAttr), "true"))
> + return true;
> + if (equalIgnoringCase(getAttribute(draggableAttr), "false"))
> + return false;
It would be more efficient to call getAttribute only once. A local variable of
type const AtomicString& can be used to hold the attribute value.
> +void HTMLElement::setDraggable(MappedAttribute* attr)
I think it would be best to give this a different name, rather than using
overloading. Perhaps it could be called parseDraggableAttribute.
> +void HTMLElement::setDraggable(const bool& value)
This should just be bool, not const bool&.
> + void setDraggable(MappedAttribute*);
This newly-added member function should be private.
This looks good, but I'm going to say review- since I'd really like to see the
name and visibility of the setDraggable function that takes an attribute
changed.
More information about the webkit-reviews
mailing list