Bug 111262 – Cue line-height property shouldn't be inherited from the video element

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Victor Carbune]

This can be reproduced with any captioned video.

[Radar WebKit Bug Importer]

<rdar://problem/13336907>

[Victor Carbune]

It looks like the problem appear each time the video is scaled (not neccessarily
fullscreen, e.g. through height and width properties)

Before I start digging even more into this, it looks now that WebKit supports
viewport values (such as the 5vh which is the default for cues) and also using
this when scaling the video seems to fix a part of the problem (no overlapping 
lines)

Firstly, I'm thinking of dropping the manual CSS font-size adjustment of cues 
that happens in TextTrackContainer::updateSizes(). Is there anything that might
block this?

[Eric Carlson]

(In reply to comment #2)
> It looks like the problem appear each time the video is scaled (not neccessarily
> fullscreen, e.g. through height and width properties)
> 
> Before I start digging even more into this, it looks now that WebKit supports
> viewport values (such as the 5vh which is the default for cues) and also using
> this when scaling the video seems to fix a part of the problem (no overlapping 
> lines)
> 
> Firstly, I'm thinking of dropping the manual CSS font-size adjustment of cues 
> that happens in TextTrackContainer::updateSizes(). Is there anything that might
> block this?

It is also possible for user preferences to override font size, eg. CaptionUserPreferencesMac::captionFontSizeScale, but there is no reason that we couldn't change the size by adding a font size override to the style sheet generated by CaptionUserPreferencesMac::captionsStyleSheetOverride. 

This would actually be simpler because it could be done only when the user preferences change (as we do with the other overrides) instead of every time the size changes.

Note, I originally tried to use vh but it did not work

[Victor Carbune]

(In reply to comment #3)
> Note, I originally tried to use vh but it did not work
You are right, it still doesn't work correctly.

Further debugging shows that this doesn't happen all the time, but particularly
when the line-height on the video element is inherited from containing blocks.

I think this is a problem with the shadow tree, and such properties shouldn't
be inherited (I'm not sure if there's a bug already for this).

[Victor Carbune]

Created attachment 191549 [details]
Fix

[Eric Carlson]

(In reply to comment #4)
> (In reply to comment #3)
> > Note, I originally tried to use vh but it did not work
> You are right, it still doesn't work correctly.
> 
  Darn :-(

> Further debugging shows that this doesn't happen all the time, but particularly
> when the line-height on the video element is inherited from containing blocks.
> 
> I think this is a problem with the shadow tree, and such properties shouldn't
> be inherited (I'm not sure if there's a bug already for this).
>
  I agree that this looks like a shadow tree problem, the existing code that sets inline style sometimes works and sometimes does not.

[WebKit Review Bot]

Comment on attachment 191549 [details]
Fix

Clearing flags on attachment: 191549

Committed r144814: <http://trac.webkit.org/changeset/144814>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.