1566199 - Subtitle selection widget not displayed with 'preload="none"'

Status: VERIFIED FIXED

(Toolkit :: Video/Audio Controls, defect, P3)

Description

[Vincent Bernat]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Load the following HTML code (no need to provide subtitle files):

<div>
    <video width="480" height="201" controls preload="none" crossorigin="anonymous">
        <source src="https://media.luffy.cx/videos/2019-self-hosted-videos-subtitles/progressive.mp4" type='video/mp4; codecs="avc1.4d401f, mp4a.40.2"'>
        <track src="2019-self-hosted-videos-subtitles.de.vtt" kind="subtitles" srclang="de" label="Deutsch">
        <track src="2019-self-hosted-videos-subtitles.en.vtt" kind="subtitles" srclang="en" label="English">
    </video>
</div>

<div>
    <video width="480" height="201" controls crossorigin="anonymous">
        <source src="https://media.luffy.cx/videos/2019-self-hosted-videos-subtitles/progressive.mp4" type='video/mp4; codecs="avc1.4d401f, mp4a.40.2"'>
        <track src="2019-self-hosted-videos-subtitles.de.vtt" kind="subtitles" srclang="de" label="Deutsch">
        <track src="2019-self-hosted-videos-subtitles.en.vtt" kind="subtitles" srclang="en" label="English">
    </video>
</div>

Actual results:

The first video doesn't show the subtitle selection widget until you press play. The second video shows the subtitle selection widget on load.

Expected results:

The first video should have shown the subtitle selection widget too. There is no need to preload the video to have the appropriate information to populate the subtitle menu.

Comment 1

[Vincent Bernat]

[:alwu] already had a look at this bug in https://bugzilla.mozilla.org/show_bug.cgi?id=1464012 (see last comment).

Comment 2

[:Gijs (he/him)]

Al, can you confirm if there are addtrack events that fire in the non-preload case, and/or why the video controls aren't picking those up? And/Or should the video controls query for available tracks somehow when they attach?

Comment 3

[Alastor Wu [:alwu]]

After a quick look, we did dispatch addtrack and video control also received that event, but somehow video control didn't show the closedCaptionButton correctly.

Comment 4

[:Gijs (he/him)]

This is because for the not-preloaded video, the video width/height is 0 and as a result of bug 1527688 that avoids showing the caption button. Looks like we might need to do a different check there?

Comment 5

[Alastor Wu [:alwu]]

(In reply to :Gijs (he/him) from comment #4)

This is because for the not-preloaded video, the video width/height is 0 and as a result of bug 1527688 that avoids showing the caption button. Looks like we might need to do a different check there?

Thank for the finding! Yes, I think we can check tag name instead to know if it's audio element or not.

Comment 6

[:Gijs (he/him)]

(In reply to Alastor Wu [:alwu] from comment #5)

(In reply to :Gijs (he/him) from comment #4)

This is because for the not-preloaded video, the video width/height is 0 and as a result of bug 1527688 that avoids showing the caption button. Looks like we might need to do a different check there?

Thank for the finding! Yes, I think we can check tag name instead to know if it's audio element or not.

I know we've had issues in the past with video elements that don't contain a video stream, only audio - perhaps we should use isAudioOnly and ensure that we update the button state in the isAudioOnly setter (so that if we decide later that the video is audio only, once we do have metadata, then we hide the subtitle button then) ?

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Happy to take a patch for 70, but since this is triaged and set to P3 priority I'm setting it as fix-optional.
That will remove the bug from weekly regression triage.

Comment 8

[Vincent Bernat]

Wouldn't it be simpler to revert the patch from bug 1527688? If the only downside of not having the patch is to let the user select subtitles while they cannot be displayed, this seems better than not being able to select subtitles while they can be displayed. The former situation also requires someone to provide subtitles for an audio-only content while they cannot be displayed.

Comment 9

[:Gijs (he/him)]

(In reply to Vincent Bernat from comment #8)

Wouldn't it be simpler to revert the patch from bug 1527688? If the only downside of not having the patch is to let the user select subtitles while they cannot be displayed, this seems better than not being able to select subtitles while they can be displayed. The former situation also requires someone to provide subtitles for an audio-only content while they cannot be displayed.

Without looking at this too deeply, this seems a reasonable suggestion - :alwu ?

Comment 10

[Alastor Wu [:alwu]]

Sorry, I've been busying on other stuffs, so I didn't make any progress on this bug. I'm good to the option for reverting the change from bug 1527688 for now, and then maybe we could figure out a better solution in the future when we have time.

Comment 11

[Vincent Bernat]

(In reply to Alastor Wu [:alwu] from comment #10)

Sorry, I've been busying on other stuffs, so I didn't make any progress on this bug. I'm good to the option for reverting the change from bug 1527688 for now, and then maybe we could figure out a better solution in the future when we have time.

Any chance you could do the revert in the coming weeks? Thanks.

Comment 12

[:Gijs (he/him)]

(In reply to Vincent Bernat from comment #11)

(In reply to Alastor Wu [:alwu] from comment #10)

Sorry, I've been busying on other stuffs, so I didn't make any progress on this bug. I'm good to the option for reverting the change from bug 1527688 for now, and then maybe we could figure out a better solution in the future when we have time.

Any chance you could do the revert in the coming weeks? Thanks.

Comment 13

[Alastor Wu [:alwu]]

Now I'm trying to use isAudioOnly to replace the current check, if everything goes well and that doesn't break any test on the try server, I will start asking for a review.

Sorry for keeping this bug unfixed for such as long while.

Comment 14

[Alastor Wu [:alwu]]

Attachment: Bug 1566199 - check 'isAudioOnly' for knowing if we have rendering area or not.

The not-preloaded video would have 0 width and height, so using width and length as a condition of showing caption or not would incorrectly exclude the not-preloaded video. Therefore, we should check isAudioOnly instead.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e7a3ae29aa0
check 'isAudioOnly' for knowing if we have rendering area or not. r=Gijs

Comment 16

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/0e7a3ae29aa0

Comment 17

[Ciprian Georgiu, Desktop QA]

I was able to reproduce this issue by following the steps from comment 0, on an affected Nightly build 2020-03-02.

The issue is verified as fixed on latest Beta 76.0b4 under macOS 10.15, Win 10 x64 and Ubuntu 18.04 x64.