Closed Bug 1329117 Opened 3 years ago Closed 3 years ago

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

Categories

(Toolkit :: Video/Audio Controls, defect)

53 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla54
Tracking Status
firefox51 --- wontfix
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- verified
firefox54 --- verified

People

(Reporter: hani.yacoub, Assigned: bechen)

References

Details

Attachments

(2 files)

Attached image 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.
Blocks: 1271765
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
Flags: needinfo?(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".
Flags: needinfo?(bechen)
(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?)
Assignee: nobody → ralin
Status: NEW → ASSIGNED
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.
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?
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
Assignee: ralin → bechen
Comment on attachment 8829303 [details]
Bug 1329117 - Run HonorUserPreferencesForTrackSelection at stable state.

https://reviewboard.mozilla.org/r/106410/#review107360
Attachment #8829303 - Flags: review?(jwwang) → review+
Blocks: 1332994
(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.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a5ab036af960
Run HonorUserPreferencesForTrackSelection at stable state. r=jwwang
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a5ab036af960
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Please request Aurora/Beta approval on this when you get a chance.
Flags: needinfo?(bechen)
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.
Flags: needinfo?(bechen)
Attachment #8829303 - Flags: approval-mozilla-aurora?
Comment on attachment 8829303 [details]
Bug 1329117 - Run HonorUserPreferencesForTrackSelection at stable state.

Fixes a problem with video CC control. Aurora53+
Attachment #8829303 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
(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?
Flags: needinfo?(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.
Flags: needinfo?(ralin)
Based on previous comment and status of bug 1332994, mark this bug verified on Fx 54.
Status: RESOLVED → VERIFIED
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 :)
Attachment #8829303 - Flags: approval-mozilla-beta?
Comment on attachment 8829303 [details]
Bug 1329117 - Run HonorUserPreferencesForTrackSelection at stable state.

subtitle display fix, let's get this in beta52.
Attachment #8829303 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
Flags: needinfo?(bechen)
(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.
Flags: needinfo?(bechen)
See Also: → 1339582
You need to log in before you can comment on or make changes to this bug.