[webkit-reviews] review denied: [Bug 171790] AX: Video/Audio Player Controls missing group container. : [Attachment 312318] Patch

[bugzilla-daemon at webkit.org]

Antoine Quint <graouts at apple.com> has denied Aaron Chu <aaron_chu at apple.com>'s
request for review:
Bug 171790: AX: Video/Audio Player Controls missing group container.
https://bugs.webkit.org/show_bug.cgi?id=171790

Attachment 312318: Patch

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




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

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

>
Source/WebCore/Modules/modern-media-controls/controls/inline-media-controls.js:
69
> +	       this.bottomControlsBar.element.setAttribute("aria-label",
UIString("Audio Controls"));

You should update the "aria-label" regardless of the value of `flag`:

this.bottomControlsBar.element.setAttribute("aria-label", flag ?
UIString("Audio Controls") : UIString("Video Controls"));

I think the easiest thing is to split this in a dedicated method like so:

_updateBottomControlsBarLabel()
{
    this.bottomControlsBar.element.setAttribute("aria-label",
this._shouldUseAudioLayout ? UIString("Audio Controls") : UIString("Video
Controls"));
}

and call this in the constructor and in the `shouldUseAudioLayout` setter once
the ivar has been set.