Bug 169675 – [Modern Media Controls] Captions do not default to Auto when language is changed

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Antoine Quint]

Steps To Reproduce:
1. Make sure the captions mode is set to “Auto” (open a video in QuickTime Player and select Auto in captions menu)
2. Go to System Preferences -> Language & Region, and change the system language to French
3. Play a video with French captions, and open the captions menu

Results:
The captions menu shows that the French subtitles are selected. It should show that Auto is selected.

[Antoine Quint]

<rdar://problem/30423369>

[Antoine Quint]

Created attachment 304501 [details]
Patch

[Eric Carlson]

Comment on attachment 304501 [details]
Patch

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

as discussed on irc, you should be able to create a test for this with internals.setUserPreferredLanguages().

> Source/WebCore/Modules/modern-media-controls/media/tracks-support.js:115
> +        const trackItem = this._textTracks()[trackIndex];
> +        const host = this.mediaController.host;
> +        const usesAutomaticTrack = host ? host.captionDisplayMode === "automatic" : false;
> +
> +        if (host) {
> +            if (trackItem === host.captionMenuOffItem && (host.captionDisplayMode === "forced-only" || host.captionDisplayMode === "manual"))
> +                return true;
> +            else if (trackItem === host.captionMenuAutomaticItem && usesAutomaticTrack)
> +                return true;
> +        }
> +
> +        return !usesAutomaticTrack && trackItem.mode !== "disabled";

I thinks this would be slightly easier to follow if you return immediately if host is null:

if (!host)
    return trackItem.mode !== "disabled";

if (trackItem === host.captionMenuOffItem && (host.captionDisplayMode === "forced-only" || host.captionDisplayMode === "manual"))
    return true;

if (trackItem === host.captionMenuAutomaticItem && usesAutomaticTrack)
    return true;

return trackItem.mode !== "disabled";

[Eric Carlson]

Comment on attachment 304501 [details]
Patch

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

>> Source/WebCore/Modules/modern-media-controls/media/tracks-support.js:115
>> +        return !usesAutomaticTrack && trackItem.mode !== "disabled";
> 
> I thinks this would be slightly easier to follow if you return immediately if host is null:
> 
> if (!host)
>     return trackItem.mode !== "disabled";
> 
> if (trackItem === host.captionMenuOffItem && (host.captionDisplayMode === "forced-only" || host.captionDisplayMode === "manual"))
>     return true;
> 
> if (trackItem === host.captionMenuAutomaticItem && usesAutomaticTrack)
>     return true;
> 
> return trackItem.mode !== "disabled";

or:

if (host && trackItem === host.captionMenuOffItem && (host.captionDisplayMode === "forced-only" || host.captionDisplayMode === "manual"))
    return true;

if (host && trackItem === host.captionMenuAutomaticItem && usesAutomaticTrack)
    return true;

return trackItem.mode !== "disabled";

[Antoine Quint]

Created attachment 304504 [details]
Patch

[Eric Carlson]

Comment on attachment 304504 [details]
Patch

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

> Source/WebCore/Modules/modern-media-controls/media/tracks-support.js:106
> +        const usesAutomaticTrack = host ? host.captionDisplayMode === "automatic" : false;

Nit: this variable is no longer necessary.

[Antoine Quint]

(In reply to comment #6)
> Comment on attachment 304504 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=304504&action=review
> 
> > Source/WebCore/Modules/modern-media-controls/media/tracks-support.js:106
> > +        const usesAutomaticTrack = host ? host.captionDisplayMode === "automatic" : false;
> 
> Nit: this variable is no longer necessary.

It is necessary in case the track that we’re querying the selected state for is neither Off or Auto but we should be selecting auto.

[WebKit Commit Bot]

Comment on attachment 304504 [details]
Patch

Clearing flags on attachment: 304504

Committed r213987: <http://trac.webkit.org/changeset/213987>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.