RESOLVED FIXED 135820
More adjustments to css width calculations of cue boxes
https://bugs.webkit.org/show_bug.cgi?id=135820
Summary More adjustments to css width calculations of cue boxes
Roger Fong
Reported 2014-08-11 16:29:51 PDT
<rdar://problem/17961164> Two more issues I neglected to fix: First off the multiplier calculations are wrong since they are based off a default font size of 10px which is not actually what we use. We use a percentage of the video size as specified in CaptionUserPreferences. The second issue is that for center aligned text, the cue box is offset from center after adjusting the width. This is because when we specify center alignment, what happens is that we actually calculate the left/top css value such that the cue looks centered. Thus we also need to adjust the left/top property value (by the difference in width / 2) if we're not centered such that the box remained in the center.
Attachments
patch (7.71 KB, patch)
2014-08-11 16:37 PDT, Roger Fong
bfulgham: review+
bfulgham: commit-queue-
Roger Fong
Comment 1 2014-08-11 16:37:25 PDT
Brent Fulgham
Comment 2 2014-08-11 16:59:49 PDT
Comment on attachment 236413 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=236413&action=review r=me, but please address the possible zero-base-font-size issue I mentioned. > Source/WebCore/html/track/TextTrackCueGeneric.cpp:82 > + double authorFontSize = videoSize.height() * cue->baseFontSizeRelativeToVideoHeight() / 100.0; Can baseFontSizeRelativeToVideoHeight ever be zero (I think it can be)? If the author could arbitrarily set this to zero, I think we should check and fall back to that minimum (10 pt) value from the CSS.
Roger Fong
Comment 3 2014-08-11 17:14:39 PDT
committed with review commented applied: http://trac.webkit.org/changeset/172421
Note You need to log in before you can comment on or make changes to this bug.