[webkit-reviews] review granted: [Bug 42001] Update media element's handling of empty 'src' attribute : [Attachment 61247] proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 42001: Update media element's handling of empty 'src' attribute
https://bugs.webkit.org/show_bug.cgi?id=42001

Attachment 61247: proposed patch
https://bugs.webkit.org/attachment.cgi?id=61247&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    if (!hasAttribute(srcAttr)) {

I'm surprised the above logic is right. Why should a missing src attribute be
treated differently from one that is empty or contains only spaces?

> +	   // If candidate does not have a src attribute, or if its src
attribute's value is the empty string ... jump down to the failed step below
> +	   if (!source->hasAttribute(srcAttr))
> +	       goto check_again;
> +	   
> +	   mediaURL = source->getNonEmptyURLAttribute(srcAttr);
> +	   if (mediaURL.isEmpty())
> +	       goto check_again;

I think you could just leave out the hasAttribute check here. It's just an
optimization for the case where there's no src attribute at all. And I don't
think that case needs to be optimized.

r=me