1329117 - The subtitle is displayed even if it's set to Off

Status: VERIFIED FIXED

(Toolkit :: Video/Audio Controls, defect)

Description

[Hani Yacoub, Desktop QA]

Attachment: subtitle.png

[Affected versions]: 
Nightly 53.0a1

[Affected platforms]:
All platforms: Windows 10 x 64, Mac OS X 10.11

[Steps to reproduce]:
1. Launch Nightly 53.0a1 with a new profile and then open this link https://people-mozilla.org/~ralin/vtt/.
2. Observe that the subtitle is displayed.
3. Click on the CC button and observe that it's set to Off.

[Expected result]:
The subtitle shouldn't be displayed if it's set to Off.

[Actual result]:
The subtitle is displayed even when it's set to Off.

[Note]:
If you click on Off button from CC, the subtitle will not be displayed any more.

Comment 1

[Ray Lin[:ralin]]

It seems that we don't get the right selected track in the initialization process. 

We determine whether the text track is `showing` or not by checking its `mode` in `addtrack` event, but I found that one of the tracks(maybe the first one) will be automatically turned on in a timing rather than `addtrack`. We used to get correct mode when `addtrack` but I'm surprise that the behavior looks different now.

Benjamin, do you know is there any change about `addtrack` event and its context since CC btn(bug 887934) landed?  Thanks

Comment 2

[Benjamin Chen [:bechen]]

https://html.spec.whatwg.org/multipage/embedded-content.html#honor-user-preferences-for-automatic-text-track-selection

Yes, the first track might be turn on automatically.

So the solution I guess should be "reorder the code flow TextTrackManager::AddTextTrack" or "run TextTrackManager::HonorUserPreferencesForTrackSelection in stable state".

Comment 3

[:Gijs (he/him)]

(In reply to Benjamin Chen [:bechen] from comment #2)
> Yes, the first track might be turn on automatically.

Is there no event that fires for this?

Why did this regress with the new video controls? (What did the old ones do?)

Comment 4

[Hani Yacoub, Desktop QA]

This issue started to appear with Firefox Nightly 51.0a1, because in the the older versions of Firefox nightly the subtitle button wasn't available.

Comment 5

[:Gijs (he/him)]

Well, this missed the 51 cutoff (we're shipping next week) so effectively we're going to ship this broken on 51. :-\

Are there tests for this feature?

Comment 6

[Ray Lin[:ralin]]

There's a test for CC btn[0]. The tracks are dynamically added to video, and their mode are set in advance.

I've discussed with :bechen, and I would move the assignee to him since he knows better about this issue. 

Thanks.

https://dxr.mozilla.org/mozilla-central/rev/3cedab21a7e65e6a1c4c2294ecfb5502575a46e3/toolkit/content/tests/widgets/test_videocontrols_vtt.html

Comment 7

[Benjamin Chen [:bechen]]

Attachment: Bug 1329117 - Run HonorUserPreferencesForTrackSelection at stable state.

Review commit: https://reviewboard.mozilla.org/r/106410/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/106410/

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8829303 [details]
Bug 1329117 - Run HonorUserPreferencesForTrackSelection at stable state.

https://reviewboard.mozilla.org/r/106410/#review107360

Comment 9

[Ray Lin[:ralin]]

(In reply to :Gijs from comment #5)
> Well, this missed the 51 cutoff (we're shipping next week) so effectively
> we're going to ship this broken on 51. :-\
> 
> Are there tests for this feature?

filed a bug 1333103 for CC menu, that should be able to ensure the entire feature.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5ab036af960
Run HonorUserPreferencesForTrackSelection at stable state. r=jwwang

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a5ab036af960

Comment 12

[Julien Cristau [:jcristau]]

Please request Aurora/Beta approval on this when you get a chance.

Comment 13

[Ray Lin[:ralin]]

Comment on attachment 8829303 [details]
Bug 1329117 - Run HonorUserPreferencesForTrackSelection at stable state.

(In reply to Julien Cristau [:jcristau] from comment #12)
> Please request Aurora/Beta approval on this when you get a chance.

Thank you Julien, I initially thought this bug should be uplifted with bug 1332994, but it's no harm to request Aurora for this first. 

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1329117
[User impact if declined]: affect the followup bug Bug 1332994(UI part). If declined, closed caption menu could not display correctly. (the enabled item doesn't highlight)
[Is this code covered by automated tests?]: will be covered in Bug 1332994
[Has the fix been verified in Nightly?]: none, hard to verify since the UI part isn't landed yet
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low risk
[Why is the change risky/not risky?]: only fix a property of `addtrack` event object, and few lines of code modification.
[String changes made/needed]: none

Thanks.

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8829303 [details]
Bug 1329117 - Run HonorUserPreferencesForTrackSelection at stable state.

Fixes a problem with video CC control. Aurora53+

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9df75a609fd4

Comment 16

[Brindusa Tot, Desktop QA]

(In reply to Ray Lin[:ralin] from comment #13)
 

> [User impact if declined]: affect the followup bug Bug 1332994(UI part). If
> declined, closed caption menu could not display correctly. (the enabled item
> doesn't highlight)
> [Is this code covered by automated tests?]: will be covered in Bug 1332994
> [Has the fix been verified in Nightly?]: none, hard to verify since the UI
> part isn't landed yet
> [Needs manual test from QE? If yes, steps to reproduce]: no
> [List of other uplifts needed for the feature/fix]: none
> [Is the change risky?]: low risk
> [Why is the change risky/not risky?]: only fix a property of `addtrack`
> event object, and few lines of code modification.
> [String changes made/needed]: none
> 
> Thanks.

I was trying to verify this issue, but as I understand the UI part of this bug will be fixed by bug 1332994. Is there a way I can verify this before the fix of bug 1332994?

Comment 17

[Ray Lin[:ralin]]

(In reply to Brindusa Tot[:brindusat] from comment #16)


Hi Brindusa,

I'm afraid that you might need to modify the code inside <video> binding to see the changes. If it is fine, I would suggest we verify bug 1332994 only as bug 1332994 is landing now.

Comment 18

[Brindusa Tot, Desktop QA]

Based on previous comment and status of bug 1332994, mark this bug verified on Fx 54.

Comment 19

[Ray Lin[:ralin]]

Comment on attachment 8829303 [details]
Bug 1329117 - Run HonorUserPreferencesForTrackSelection at stable state.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 887934
[User impact if declined]: get incorrect closed caption menu
[Is this code covered by automated tests?]: is covered in Bug 1332994
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low risk
[Why is the change risky/not risky?]: few lines of code modification.
[String changes made/needed]: none

Thanks :)

Comment 20

[Julien Cristau [:jcristau]]

Comment on attachment 8829303 [details]
Bug 1329117 - Run HonorUserPreferencesForTrackSelection at stable state.

subtitle display fix, let's get this in beta52.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/15d8f9d49bfe

Comment 22

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox  53.0a1 (2016.01.06) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 53.0a2, 54.0a1 on Win 8.1 x64, Ubuntu 16.04 x64, and Mac 10.11.
Firefox 52.0b5 on Win 8.1 x64, Ubuntu 16.04 x64 and Mac OS X 10.11 are still affected.

Comment 23

[Ray Lin[:ralin]]

(In reply to Timea Zsoldos from comment #22)
> I have reproduced this issue using Firefox  53.0a1 (2016.01.06) on Win 8.1
> x64.
> I can confirm this issue is fixed, I verified using Firefox 53.0a2, 54.0a1
> on Win 8.1 x64, Ubuntu 16.04 x64, and Mac 10.11.
> Firefox 52.0b5 on Win 8.1 x64, Ubuntu 16.04 x64 and Mac OS X 10.11 are still
> affected.

Thank you Timea,

Answer the question for :bechen. As I mentioned in comment 13, this bug should uplift with Bug 1332994. I was planning to uplift both in the same day, however, I got some problem delaying uplift Bug 1332994 and that is why this problem is still in Beta.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/15d8f9d49bfe