Summary: | Determine parameters for rendering a TextTrackCue | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Victor Carbune <vcarbune> | ||||
Component: | Media | Assignee: | Victor Carbune <vcarbune> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dglazkov, eric.carlson, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 79347, 79750 | ||||||
Attachments: |
|
Description
Victor Carbune
2012-02-27 23:12:19 PST
Created attachment 132011 [details]
Preliminary patch
Implemented the computational steps described in the spec. The patch could be submitted stand-alone - as it is - or in the same time with the first implemented rendering case that is blocked by it.
Comment on attachment 132011 [details] Preliminary patch Attachment 132011 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11954572 Comment on attachment 132011 [details] Preliminary patch View in context: https://bugs.webkit.org/attachment.cgi?id=132011&action=review This is close, but it needs some clean-up and tests. > Source/WebCore/html/track/TextTrackCue.cpp:128 > + m_displayWritingModeMap[Horizontal] = CSSValueHorizontalTb; > + m_displayWritingModeMap[VerticalGrowingLeft] = CSSValueVerticalLr; > + m_displayWritingModeMap[VerticalGrowingRight] = CSSValueVerticalRl; Nit: it would be good to have a comment here about why these are correct defaults. > Source/WebCore/html/track/TextTrackCue.cpp:438 > + int maximumSize; This will remain uninitialized if none of the conditions below evaluate to true. Even if this can't logically happen, the compiler will complain. > Source/WebCore/html/track/TextTrackCue.cpp:456 > + m_displaySize = m_cueSize < maximumSize ? m_cueSize : maximumSize; std::min() ? > Source/WebCore/html/track/TextTrackCue.cpp:460 > + // width be 'sizeâvw' and height be 'auto'. Otherwise, let width be 'auto' > + // and height be 'sizeâvh'. (These are CSS values used by the next section Nit: "sizeâvw" and "sizeâvh" ? > Source/WebCore/html/track/TextTrackCue.cpp:504 > + // 10.10 Let left be 'x-positionâvw' and top be 'y-positionâvh'. Nit: copy pasta here as well > Source/WebCore/html/track/TextTrackCue.h:161 > + int m_displayWritingModeMap[3]; WritingDirection is a private enum, so I think we should add a sentinel terminal value and use it here instead of hard coding the size of this array. Submitted as part of https://bugs.webkit.org/show_bug.cgi?id=79750 Changeset 114640 (http://trac.webkit.org/changeset/114640) |