[Webkit-unassigned] [Bug 53556] Adding Feature Defines for <track>

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=53556


Eric Carlson <eric.carlson at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #88361|review?                     |review+
               Flag|                            |




--- Comment #16 from Eric Carlson <eric.carlson at apple.com>  2011-04-06 09:40:58 PST ---
(From update of attachment 88361)
r+ with the changes suggested below.

View in context: https://bugs.webkit.org/attachment.cgi?id=88361&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

Shouldn't leave an OOPS in the ChangeLog. It would be helpful to say why no new tests were added with this patch.


> Source/WebCore/html/HTMLTrackElement.cpp:66
> +        // TODO(annacc):
> +        // static_cast<HTMLMediaElement*>(parentNode())->trackWasAdded(this);

This isn't right, maybe you mean something like "trackWillBeRemoved(this)"?


> Source/WebCore/html/HTMLTrackElement.cpp:114
> +bool HTMLTrackElement::isDefault() const
> +{
> +    return equalIgnoringCase(getAttribute(defaultAttr), "true");
> +}

"default" is a boolean attribute, so I think you should use "return hasAttribute(defaultAttr)"


> Source/WebCore/html/HTMLTrackElement.cpp:119
> +void HTMLTrackElement::setIsDefault(bool isDefault)
> +{
> +    setAttribute(defaultAttr, isDefault ? "true" : "false");
> +}

And here you can use "setBooleanAttribute(defaultAttr, isDefault);"

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.