Closed Bug 985915 Opened 6 years ago Closed 3 years ago

[webvtt] The two rows subtitles are overlapped with video controls on mouse hover

Categories

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

29 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox31 --- ?
firefox50 --- fixed

People

(Reporter: petruta.rasa, Assigned: ralin)

References

(Blocks 1 open bug, )

Details

Attachments

(2 files)

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0
Reproducible on latest Firefox 29 beta 1 (20140318013849) and latest Aurora 30.0a2 (20140319004002).
Not reproducible on latest Nightly 31.0a1 (20140319030201) where the subtitles are missing.

Steps to reproduce:
1. Open the link with the demo video: http://rickeyre.ca/2013/06/20/webvtt-cycle-collector-demo.html
2. Start the video
3. Hover the mouse over the video area

Actual results:
The subtitles are overlapped by the video controls

Expected results:
The subtitle area should be placed above the controls when the video is played inline (just as they are when video is played full screen)

Notes:
1. Reproducible also on Mac OS X 10.8 and Ubuntu 32-bit.
2. The issue was introduced in Firefox 29 when the feature landed, so it's not a regression. Will continue investigate to see when it was introduced.
The spec doesn't say anything about this as far as I know. It does say that the implementor has leeway in determining the padding around the edges of the video though, so we could add more to the bottom of the box. I'm not sure what the best decision would be. I see a lot of subtitle implementations that have the subtitles low enough for the controls to overlap it when they pop up.
Component: Audio/Video → Audio/Video: Playback
Assignee: nobody → alwu
In [1] step4, it mentions that we should prepare "one or more completely transparent positioned CSS block boxes that cover the same region as the user interface".

[1] https://w3c.github.io/webvtt/#processing-model
Depends on: 887934
This is fixed by the patches in bug 8878934.
The bug 8878934 is about adding the switch for subtitle, this bug would handle the overlapping problem.
Ray already has a patch for that.
Assignee: alwu → ralin
Depends on: 882718
(In reply to Alastor Wu [:alwu] from comment #4)
> The bug 8878934 is about adding the switch for subtitle, this bug would
> handle the overlapping problem.
> Ray already has a patch for that.

Yes, that's true but the part 2 patch in that bug does fix this, unless there is some more work planned.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> (In reply to Alastor Wu [:alwu] from comment #4)
> > The bug 8878934 is about adding the switch for subtitle, this bug would
> > handle the overlapping problem.
> > Ray already has a patch for that.
> 
> Yes, that's true but the part 2 patch in that bug does fix this, unless
> there is some more work planned.

Thanks :jaws

I discussed with :alwu offline, and we have not got a clear picture of how to implement the behavior[1] of avoiding overlapping problem then. Therefore, In Bug 887934, I'll push a UI patch(css + xml) later, and transfer Part 2 to this bug. 
I think it would be better to do so to not block UI part before aligning with spec.

[1] https://w3c.github.io/webvtt/#processing-model  rule 4 & 5
(In reply to Ray Lin[:ralin] from comment #6)
> Therefore, In Bug 887934, I'll push a UI patch(css + xml) later, and
> transfer Part 2 to this bug.
How closely related to this is bug 992664? Someone mentioned this hover problem in that bug.
(In reply to Mardeg from comment #7)
> How closely related to this is bug 992664? Someone mentioned this hover
> problem in that bug.

It seems like a different bug, this bug is to handle the cue overlapping problem.
However, in bug 992664, I can't see any cues in the video even I turn off its the control interface.
Ray, is there an update here? This needs to land in the same release as bug 887934.
Flags: needinfo?(ralin)
No problem.

I got an almost done patch in hand. As bug 887934 just successfully landed(tested), here's something I'll do next:
- rebase and maybe solve conflict
- push to mozreview and ask :rillian to review again

This could be landed the earlier next week if nothing goes wrong. Thanks for your reminder.
Flags: needinfo?(ralin)
Comment on attachment 8765325 [details]
Bug 985915 - Vertically move up closed caption to not overlap control bar.

https://reviewboard.mozilla.org/r/60766/#review57722

r=me with the webvtt_displaystate test failures addressed.
Attachment #8765325 - Flags: review?(giles) → review+
Comment on attachment 8765325 [details]
Bug 985915 - Vertically move up closed caption to not overlap control bar.

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

fix test failures. adding a conditional statment in case no control bar present.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c69c648e7b9
Keywords: checkin-needed
Status: NEW → ASSIGNED
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6718c7f1486
Vertically move up closed caption to not overlap control bar. r=rillian
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d6718c7f1486
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.