[webkit-reviews] review granted: [Bug 42087]=?UTF-8?Q?=20Don=E2=80=99t=20handle=20empty=20URL=20attributes=20specially=20in=20reflection?=, even in cases such as <img src=""> : [Attachment 61594] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Darin Adler
<darin at apple.com>'s request for review:
Bug 42087: Don’t handle empty URL attributes specially in reflection, even in
cases such as <img src="">
https://bugs.webkit.org/show_bug.cgi?id=42087

Attachment 61594: Patch
https://bugs.webkit.org/attachment.cgi?id=61594&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
>  
>  Set 'poster' to ''.
>  EXPECTED (video.getAttribute('poster') == '') OK
> -EXPECTED (relativeURL(video.poster) == '') OK
> +EXPECTED (relativeURL(video.poster) == 'video-poster.html') OK
>  
I understand that this is what the spec says  is "correct", but it makes no
sense at all. What user expects video.poster to be the document url?

> -EVENT(error) from <source id='empty-src' src=''> OK
> +EVENT(error) from <source id='empty-src'
src='video-source-error-no-candidate.html'> OK
>  EXPECTED (video.error == 'null') OK
Ditto.


> Index: LayoutTests/media/video-src-none-expected.txt
> ===================================================================
> --- LayoutTests/media/video-src-none-expected.txt	(revision 63382)
> +++ LayoutTests/media/video-src-none-expected.txt	(working copy)
> @@ -8,7 +8,7 @@ EXPECTED (videos[0].src == '') OK
>  ** <video> with empty src attribute **
>  EXPECTED (videos[1].error == 'null') OK
>  EXPECTED (videos[1].networkState == '0') OK
> -EXPECTED (videos[1].src == '') OK
> +EXPECTED (videos[1].src == 'relativeURL('video-src-none.html')'), OBSERVED
'file:///Volumes/Home/darin/Safari/OpenSource/LayoutTests/media/video-src-none.
html' FAIL
>  
Probably not good to check in the result with a failure, or with a path to a
file on your machine.


r=me with a fix for the failure above, although I strongly disagree with the
spec's mandated behavior. We should get this changed.