[webkit-reviews] review denied: [Bug 110263] Add the default video poster if it doesn't exist in video tag : [Attachment 189368] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has denied michaelbai at chromium.org's
request for review:
Bug 110263: Add the default video poster if it doesn't exist in video tag
https://bugs.webkit.org/show_bug.cgi?id=110263

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

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=189368&action=review


Marking r- because of the GTK problem.

> LayoutTests/media/video-default-poster.html:17
> +	     document.getElementById("result-no-poster").innerText =
> +		 poster.value == default_poster ? "PASS" : "FAIL";

Nit: I think the line break + ternary make this more difficult to understand
instead of easier.

> LayoutTests/media/video-default-poster.html:20
> +	 } else {
> +	     document.getElementById("result-no-poster").innerText = "FAIL:
poster attribute is null";
> +	 }

Nit: single line if conditionals don't need braces.

> LayoutTests/media/video-default-poster.html:27
> +	 if (poster) {
> +	     document.getElementById("result-has-poster").innerText =
> +		 poster.value == "content/abe.png" ? "PASS" : "FAIL: poster was
changed";
> +	 } else {
> +	     document.getElementById("result-has-poster").innerText = "FAIL:
poster attribute is null";
> +	 }

Dittos.

> LayoutTests/media/video-no-default-poster.html:21
> +	 document.getElementById("result-no-poster").innerText =
> +	     poster ? "FAIL : poster was added." : "PASS";
> +	 poster =
document.getElementById("video-has-poster").getAttributeNode("poster");
> +	 if (poster) {
> +	     document.getElementById("result-has-poster").innerText =
> +		 poster.value == "content/abe.png" ? "PASS" : "FAIL: poster was
changed";
> +	 } else {
> +	     document.getElementById("result-has-poster").innerText = "FAIL:
poster attribute is null";
> +	 }

Dittos.