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

Status: RESOLVED FIXED

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

Description

[Petruta Horea [:phorea], Desktop QA]

Attachment: vtt subtitle overlaps controls.jpg

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.

Comment 1

[Rick Eyre (:reyre)]

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.

Comment 2

[Alastor Wu [: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

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

This is fixed by the patches in bug 8878934.

Comment 4

[Alastor Wu [:alwu]]

The bug 8878934 is about adding the switch for subtitle, this bug would handle the overlapping problem.
Ray already has a patch for that.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

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

Comment 6

[Ray Lin[:ralin]]

(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

Comment 7

[Mardeg]

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

Comment 8

[Alastor Wu [:alwu]]

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

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Ray, is there an update here? This needs to land in the same release as bug 887934.

Comment 10

[Ray Lin[: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.

Comment 11

[Ray Lin[:ralin]]

Attachment: Bug 985915 - Vertically move up closed caption to not overlap control bar.

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

Comment 12

[Ralph Giles (:rillian)]

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.

Comment 13

[Ray Lin[:ralin]]

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/

Comment 14

[Ray Lin[:ralin]]

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6718c7f1486
Vertically move up closed caption to not overlap control bar. r=rillian

Comment 16

[Carsten Book [:Tomcat]]

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