[webkit-reviews] review denied: [Bug 191574] Include tv name in AirPlay placard. : [Attachment 354617] Patch

[bugzilla-daemon at webkit.org]

Antoine Quint <graouts at apple.com> has denied Jeremy Jones
<jeremyj-wk at apple.com>'s request for review:
Bug 191574: Include tv name in AirPlay placard.
https://bugs.webkit.org/show_bug.cgi?id=191574

Attachment 354617: Patch

https://bugs.webkit.org/attachment.cgi?id=354617&action=review




--- Comment #9 from Antoine Quint <graouts at apple.com> ---
Comment on attachment 354617
  --> https://bugs.webkit.org/attachment.cgi?id=354617
Patch

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

I think we can now remove "This video is playing on your Apple TV" from the
localized string and remove the description set in the AirplayPlacard
constructor.

r- because of the test regressions.

> Source/WebCore/Modules/modern-media-controls/media/placard-support.js:77
> +	   var type = this.mediaController.host.externalDeviceType;

We should be using a switch here, assuming there's a known list of options
returned by externalDeviceType.

> Source/WebCore/Modules/modern-media-controls/media/placard-support.js:79
> +	       deviceName = UIString("This video is playing on â%sâ.",
this.mediaController.host.externalDeviceDisplayName || "Apple TV");

I think "Apple TV" needs to be added as a localized string.

> Source/WebCore/en.lproj/modern-media-controls-localized-strings.js:3
> +    "This video is playing on the TV.": "This video is playing on the TV.",

These should be alphabetically sorted.