Closed Bug 1566199 Opened 1 year ago Closed 8 months ago

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

Categories

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

70 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla76
Tracking Status
firefox-esr68 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox74 --- wontfix
firefox75 --- wontfix
firefox76 --- verified

People

(Reporter: bernat, Assigned: alwu)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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.

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

Component: Untriaged → Video/Audio Controls
Product: Firefox → Toolkit
Version: 68 Branch → 70 Branch

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?

Flags: needinfo?(alwu)
Priority: -- → P3

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.

Flags: needinfo?(alwu)

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?

Flags: needinfo?(alwu)
Regressed by: 1527688

(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.

Assignee: nobody → alwu
Flags: needinfo?(alwu)

(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) ?

Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

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.

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.

(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 ?

Flags: needinfo?(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.

Flags: needinfo?(alwu)

(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.

(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.

Flags: needinfo?(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.

Flags: needinfo?(alwu)

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.

Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0e7a3ae29aa0
check 'isAudioOnly' for knowing if we have rendering area or not. r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
Flags: qe-verify+

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.