1364185 - Captions do not disappear when setting TextTrack.mode to 'hidden'

Status: RESOLVED FIXED

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

Description

[Andre Desroches]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Steps to reproduce:

1) Open a page with a video tag with captions, e.g:
https://thepaciellogroup.github.io/AT-browser-tests/test-files/video.html
2) Enable captions using the native controls
3) Inspect the video element
4) Set $0.textTracks[0].mode = 'hidden' in the console.



Actual results:

Captions remain visible.

They will disappear when hovering over the video causes the UI to re-draw when the controls become visible.


Expected results:

Captions should be hidden immediately upon setting mode to hidden. This is especially problematic when the native controls are disabled, in that case it only seems to re-draw and hide the captions when the video is seeked or when the cue changes.

Note that setting mode to 'disabled' doesn't have the same issue.

Comment 1

[Flore Allemandou [:flore]]

It works as expected (as you described in your expected results) for me in Nightly (55.0a1 (2017-05-10) (64 bits)).

The caption immediately disappears when I enter $0.textTracks[0].mode = 'hidden' in the console while the video is playing. No need to hover over the video, I did not move the cursor.

Comment 2

[Loic]

WFM with FF53 on Win 7.

Comment 3

[Andre Desroches]

Sorry to clarify this happens for me on 53.0.2 (64-bit) on Mac (MacOS Sierra 10.12.4)

Comment 4

[Flore Allemandou [:flore]]

(In reply to Andre Desroches from comment #3)
> Sorry to clarify this happens for me on 53.0.2 (64-bit) on Mac (MacOS Sierra
> 10.12.4)

Can you check with a fresh profile : https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles
If it works on a fresh profile, then it's probably one of your addons that is the issue.

If it does not work, can you test on the latest Nightly (https://www.mozilla.org/fr/firefox/channel/desktop/#nightly). If it works on the latest nightly, then the bug will be closed as "WORKSFORME".

Comment 5

[Andre Desroches]

I tried with a new profile and in the latest nightly 55.0a1 (2017-05-12) (64-bit) on MacOS and the issue still occurs.

It's easier to repro if you pause the video before setting the mode to hidden.

Comment 6

[Flore Allemandou [:flore]]

I checked, if the video is paused, then of course the caption does not disappear when you change the value to hidden.

If it had not changed after restart or immediately when the video is playing, I would have understood the bug. But in this particular case, I don't see the problem. The caption disappear when the video plays, as expected.


Can you explain the usecase of wanting the caption to disappear from a frame in a paused video? As long as they disappear when you restart the video (which they do)?

Comment 7

[Andre Desroches]

From the W3C Video Element spec (https://www.w3.org/TR/2011/WD-html5-20110113/video.html):

Hidden
Indicates that the text track is active, but that the user agent is not actively displaying the cues.

So the captions are expected to not be visible when hidden, regardless of whether the video is playing/paused/restarted. This is how other browsers implemented the spec so developers/users would expect Firefox to behave similarly, rather than have developers implement workarounds.

Comment 8

[Flore Allemandou [:flore]]

OK, I confirm the bug. In that Firefox's behavior is what you describe when the video is paused
As to whether it is the expected behavior or not, I don't know.

Comment 9

[Blake Wu [:bwu][:blakewu]]

Benjamin,
Can you help check it?

Comment 10

[Benjamin Chen [:bechen]]

Attachment: Bug 1364185 - Call DispatchUpdateCueDisplay() in TextTrackManager::NotifyCueUpdated.

If the video is paused, then set the "TextTrack.mode=hidden", in this case, TimeMarchesOn will return at step 7,
so the hidden cue still remain on the screen. So call DispatchUpdateCueDisplay explicitly.

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

Comment 11

[Alastor Wu [:alwu]]

Comment on attachment 8867614 [details]
Bug 1364185 - Call DispatchUpdateCueDisplay() in TextTrackManager::NotifyCueUpdated.

https://reviewboard.mozilla.org/r/139184/#review142470

::: dom/html/TextTrackManager.cpp:830
(Diff revision 1)
>  TextTrackManager::NotifyCueUpdated(TextTrackCue *aCue)
>  {
>    // TODO: Add/Reorder the cue to mNewCues if we have some optimization?
>    WEBVTT_LOG("NotifyCueUpdated");
>    DispatchTimeMarchesOn();
> +  // For the case "Texttrack.mode = hidden", the TimeMarchesOn

nits : We should modify this comment, it's not just about "hidden", because the display would also not be updated immediately when we change mode from "hidden" to "showing".

Comment 12

[Benjamin Chen [:bechen]]

Comment on attachment 8867614 [details]
Bug 1364185 - Call DispatchUpdateCueDisplay() in TextTrackManager::NotifyCueUpdated.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139184/diff/1-2/

Comment 13

[Benjamin Chen [:bechen]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fcc709d161ca

Comment 14

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev 527517360a48 needs "Bug N" or "No bug" in the commit message.
remote: bechen <[e-mail]>
remote: Bug1364185 - Call DispatchUpdateCueDisplay() in TextTrackManager::NotifyCueUpdated. r=alwu
remote: 
remote: If the video is paused, then set the "TextTrack.mode=hidden/showing", in this case, TimeMarchesOn will return at step 7,
remote: so the hidden cue still remain on the screen. So call DispatchUpdateCueDisplay explicitly to update the screen.
remote: 
remote: MozReview-Commit-ID: IuazaeSXX31
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote

Comment 15

[Benjamin Chen [:bechen]]

Comment on attachment 8867614 [details]
Bug 1364185 - Call DispatchUpdateCueDisplay() in TextTrackManager::NotifyCueUpdated.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139184/diff/2-3/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2dc093cc8d32
Call DispatchUpdateCueDisplay() in TextTrackManager::NotifyCueUpdated. r=alwu

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2dc093cc8d32