Closed Bug 1464012 Opened 7 years ago Closed 5 years ago

[webvtt] Video element does not load subtitles track if initialized with preload attribute set to none

Categories

(Core :: Audio/Video: Playback, defect, P3)

Product:
Core
Component:
Audio/Video: Playback
Version:
60 Branch
Platform:
Unspecified
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla69
Tracking Flags:
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: marek.raida, Assigned: alwu)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(8 files)

shouldWork.html
622 bytes, text/html
Details
FFcloneFix.html
1.08 KB, text/html
Details
FFpreloadFix.html
607 bytes, text/html
Details
preload.html
376 bytes, text/html
Details
preload.html
331 bytes, text/html
Details
Screenshot of network monitor after loading the page
222.30 KB, image/jpeg
Details
Bug 1464012 - part1 : directly check media element's crossorigin attribute.
47 bytes, text/x-phabricator-request
jcristau
: approval-mozilla-beta-
Details | Review
Bug 1464012 - part2 : add test 'test_texttrack_cors_preload_none.html'.
47 bytes, text/x-phabricator-request
jcristau
: approval-mozilla-beta-
Details | Review
Attached file shouldWork.html —
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Firefox/60.0 Build ID: 20180516032328 Steps to reproduce: Initialized video element with preload=none and subtitle track Actual results: on manual play action, video loads and plays as expected, but without loading and showing subtitle track Expected results: it should play with subtitle track as well. it does in other browsers. so it does in firefox when preload attribute is set to metadata or auto/is missing/ it could be worked around by cloning track element, but it is strange . other testcases attached in other files
Attached file FFcloneFix.html —
Attached file FFpreloadFix.html —
Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core
Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=148f39b53d9cdceaf32f1474b6bb7369f7cb1ea3&tochange=87d88eed04b26d799285a287a06fac287e2a88e6 Regressed by: 0b21c30d0edc Henry Chang — Bug 1313711 - Fix track element CORS bug. r=dveditz @Henry Chang, Your patch seems to cause the regression. Can you please look into this issue?
Blocks: CVE-2017-5408
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
status-firefox60: --- → wontfix
status-firefox61: --- → affected
status-firefox62: --- → affected
status-firefox-esr52: --- → wontfix
status-firefox-esr60: --- → affected
Ever confirmed: true
Flags: needinfo?(hchang.mozilla)
Keywords: regression
OS: Unspecified → All
Looks like Henry may not be around anymore to look at this. Will leave the ni open, but if anyone else wants this, have at it.
Priority: -- → P3
Flags: needinfo?(hchang.mozilla)

Hey!

Some additional notes about this bug:

  • With "preload=none", all subtitle tracks are loaded.
  • With "preload=none", the subtitle button doesn't appear until the user starts playing the video.

I am attaching a test file. As subtitles seem to be subject to CORS restriction, I am using a fake name for the subtitles. Check in the network monitor the file is attempted to load right on load.

Attached file preload.html —
Attached file preload.html —

Same file, but with a "local" subtitle file.

Hi, Vincent,

Sorry I'm little confused right now, which test is the one you would like us to investigate?
The tests in comment6 and 7 looks good to me, I didn't see any problem.

For the test in comment6, because we didn't start loading anything, we can't know whether this video has any subtitle or not, so we won't show the caption button before playing. For the test in comment7, it seems the file was blocking by COR, and which seems reasonable.

Thank you.

Flags: needinfo?(bernat)

For the first test, there is no need to load anything to know if there are subtitles as they are specified as tracks directly in the <video> element with all the required infos to have a menu.

To make the second test easier to understand, I have modified it this way:

<html>
<body>

    <video preload="none" controls crossorigin="anonymous">
        <source src="https://thv1.uloz.to/3/a/c/3ac76bf4c0477c1919e4934c6a611c5a.240.mp4?fileId=212959490">
        <track src="subtitles1.vtt" srclang="cz" label="České" kind="subtitles">
        <track src="subtitles2.vtt" srclang="fr" label="français" kind="subtitles">
        <track src="subtitles3.vtt" srclang="de" label="Deutsch" kind="subtitles">
        <track src="subtitles4.vtt" srclang="en" label="English" kind="subtitles">
    </video>

</body>
</html>

In the network monitor, I can see Firefox loading the 4 subtitle files while I didn't select any subtitle (and I can't select one before playing the video as the widget is not available yet).

As a comparison, Chromium is displaying the subtitle widget without loading the video and doesn't download a subtitle file before the user select a track.

Flags: needinfo?(bernat)

Could you try to use latest Nightly to see if the problem still exists?

Now we won't start loading any track which mode is disabled after landing Bug1550633, so I think those tracks won't be downloaded in this situation.

But for your another problem, showing subtitle selection in video control, it seems an enhencement for the video control, if that is your main problem, I would switch this bug to another component to which video control related bugs should belong.

Thank you.

Flags: needinfo?(bernat)

The problem about all the subtitles being loaded is not present in the latest Nightly. So only the subtitle selector being absent with "preload=none" is a problem now.

Thanks!

Flags: needinfo?(bernat)

I checked the comment0 again, so this bug should be used for solving the problem that the track couldn't be loaded successfully when setting preload=none for media element.

The reason we failed to download the resource is because we use GetCORSMode() [1] to query media element's CORS mode, which hasn't been set when setting preload=none. It would only be set when media element starts loading resource [2].

According to the spec [3], if track element's parent is media element and then we should check media element's crossorigin content attribute to decide the corsAttributeState. We should directly check its crossorigin attribue, instead of calling GetCORSMode().

[1] https://searchfox.org/mozilla-central/rev/c606cdd6d014fee4034da1702d484c0d41b604c9/dom/html/HTMLTrackElement.cpp#316
[2] https://searchfox.org/mozilla-central/rev/c606cdd6d014fee4034da1702d484c0d41b604c9/dom/html/HTMLMediaElement.cpp#2507
[3] https://html.spec.whatwg.org/multipage/media.html#sourcing-out-of-band-text-tracks:attr-media-crossorigin

Assignee: nobody → alwu
Blocks: webvtt
Summary: Video element does not load subtitles track if initialized with preload attribute set to none → [webvtt] Video element does not load subtitles track if initialized with preload attribute set to none

As GetCORSMode() would only be valid after media element starts loading, so we would fail to get the CORS mode if media's preload is none.

Therefore, according to the spec [1], we should directly check parent media element's crossorigin attribute in order to decide the corsAttributeState.

[1] https://html.spec.whatwg.org/multipage/media.html#sourcing-out-of-band-text-tracks:attr-media-crossorigin

Pushed by alwu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/105df24836a9 part1 : directly check media element's crossorigin attribute. r=jya https://hg.mozilla.org/integration/autoland/rev/8c4a3e9cc5ed part2 : add test 'test_texttrack_cors_preload_none.html'. r=jya

https://hg.mozilla.org/mozilla-central/rev/105df24836a9
https://hg.mozilla.org/mozilla-central/rev/8c4a3e9cc5ed

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox69: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Is this worth considering for Beta backport ahead of the next ESR or can it ride the trains with 69?

status-firefox65: fix-optional → wontfix
status-firefox66: fix-optional → wontfix
status-firefox67: --- → wontfix
status-firefox68: --- → fix-optional
status-firefox-esr60: affected → wontfix
Flags: needinfo?(alwu)
Flags: in-testsuite+

Comment on attachment 9071677 [details]
Bug 1464012 - part1 : directly check media element's crossorigin attribute.

Beta/Release Uplift Approval Request

  • User impact if declined: The CORS video subtitle couldn't be loaded correctly when user set video's prelod to none. It's a regression caused by bug 1313711.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Visit the page in comment0 to see if subtitle can be showed correctly.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This change is only affected on those video with preload none and its child track element has CORS resouce, it's not a common case. In addition, we have added an automation test for that.
  • String changes made/needed:
Flags: needinfo?(alwu)
Attachment #9071677 - Flags: approval-mozilla-beta?
Attachment #9071678 - Flags: approval-mozilla-beta?

Comment on attachment 9071677 [details]
Bug 1464012 - part1 : directly check media element's crossorigin attribute.

this is a longstanding issue, for a corner case, I'd rather let it ride the trains

Attachment #9071677 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9071678 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Alastor, thank you for fixing this. Given the lack of testing for compliance with fetch & security, I think we also need a test case for the invert, where the server does not respond with the correct CORS headers. Could you please take this as a follow-up?

Flags: needinfo?(alwu)

Hi, I'm happy to help, but I'm not sure what kinds of test you were mentioning, because we have media test for it in patch2 already. Could you tell me more about the test you want?
Thank you.

Flags: needinfo?(alwu) → needinfo?(fbraun)

I think he means a similar test but with the video and track looking like the below instead, such that the server doesn't respond with cors headers for the cross-origin request, and the media element shouldn't be allowed access to the vtt file.

<video preload="none" crossorigin="anonymous">
  <track src="http://example.com/tests/dom/media/test/basic.vtt" kind="subtitles" id="default" default>
</video>
Blocks: 1562353

Thanks Andreas.
Yeah, though we want a testcase with values auto and metadata for the preload attribute, don't we?

Flags: needinfo?(fbraun)

For me, it still doesn't work with the latest Nightly. Is is expected?

My test case:

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

The difference between the first and second video is preload="none". The first video doesn't have the subtitle selection widget. The second one has it. The first video will get the subtitle selection widget when you press play. Also, the second video doesn't request the subtitles unless you select one.

Adding default to one of the track in the first video downloads the track even with preload="none", but still no subtitle selection widget until we press play. So, it seems to be purely a bug with how the track selection widget is displayed.

(In reply to Vincent Bernat from comment #25)

The difference between the first and second video is preload="none". The first video doesn't have the subtitle selection widget. The second one has it. The first video will get the subtitle selection widget when you press play. Also, the second video doesn't request the subtitles unless you select one.

As you didn't set any default track for the second video, none of tracks would be automatically selected.

(In reply to Vincent Bernat from comment #26)

Adding default to one of the track in the first video downloads the track even with preload="none", but still no subtitle selection widget until we press play. So, it seems to be purely a bug with how the track selection widget is displayed.

preload is only controlling how media element loads its media resource, that doesn't mean it would stop loading text resource for another element. It seems that the video control listens addtrack [2] event to show the caption control widget, in the second video, we indeed dispatch this event, so I guess maybe we initialize video too late which results in missing the event?

You can file another bug under Toolkit :: Video/Audio Controls component, because it seems a different issue from the origin one we solved.

Thank you.

[1] https://html.spec.whatwg.org/multipage/media.html#attr-media-preload
[2] https://searchfox.org/mozilla-central/rev/1097f59d0bc17f6f8f805325c2f607e60cf0d26b/toolkit/content/widgets/videocontrols.js#1954

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: