[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