Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(355)

Unified Diff: Source/core/html/track/TextTrackCue.cpp

Issue 25155003: Fix cue rendering test and include support for left/right alignment. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Addressed comments Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/html/track/TextTrackCue.cpp
diff --git a/Source/core/html/track/TextTrackCue.cpp b/Source/core/html/track/TextTrackCue.cpp
index f80d78cbc5f8fc8bff2c3be130ea49516817f4c0..f102f1b56b69e684f8556798f0de5a1167819e16 100644
--- a/Source/core/html/track/TextTrackCue.cpp
+++ b/Source/core/html/track/TextTrackCue.cpp
@@ -73,6 +73,18 @@ static const String& endKeyword()
return end;
}
+static const String& leftKeyword()
+{
+ DEFINE_STATIC_LOCAL(const String, left, ("left"));
+ return left;
+}
+
+static const String& rightKeyword()
+{
+ DEFINE_STATIC_LOCAL(const String, right, ("right"));
+ return right;
+}
+
static const String& horizontalKeyword()
{
return emptyString();
@@ -149,12 +161,7 @@ void TextTrackCueBox::applyCSSProperties(const IntSize&)
// be set to the value in the second cell of the row of the table below
// whose first cell is the value of the corresponding cue's text track cue
// alignment:
- if (m_cue->align() == startKeyword())
- setInlineStyleProperty(CSSPropertyTextAlign, CSSValueStart);
- else if (m_cue->align() == endKeyword())
- setInlineStyleProperty(CSSPropertyTextAlign, CSSValueEnd);
- else
- setInlineStyleProperty(CSSPropertyTextAlign, CSSValueCenter);
+ setInlineStyleProperty(CSSPropertyTextAlign, m_cue->getCSSAlignment());
if (!m_cue->snapToLines()) {
// 10.13.1 Set up x and y:
@@ -219,6 +226,12 @@ TextTrackCue::TextTrackCue(ScriptExecutionContext* context, double start, double
m_displayWritingModeMap[Horizontal] = CSSValueHorizontalTb;
m_displayWritingModeMap[VerticalGrowingLeft] = CSSValueVerticalRl;
m_displayWritingModeMap[VerticalGrowingRight] = CSSValueVerticalLr;
+
+ m_displayAlignmentMap[Start] = CSSValueStart;
+ m_displayAlignmentMap[Middle] = CSSValueCenter;
+ m_displayAlignmentMap[End] = CSSValueEnd;
+ m_displayAlignmentMap[Left] = CSSValueLeft;
+ m_displayAlignmentMap[Right] = CSSValueRight;
}
TextTrackCue::~TextTrackCue()
@@ -435,6 +448,10 @@ const String& TextTrackCue::align() const
return middleKeyword();
case End:
return endKeyword();
+ case Left:
+ return leftKeyword();
+ case Right:
+ return rightKeyword();
default:
ASSERT_NOT_REACHED();
return emptyString();
@@ -456,6 +473,10 @@ void TextTrackCue::setAlign(const String& value, ExceptionState& es)
alignment = Middle;
else if (value == endKeyword())
alignment = End;
+ else if (value == leftKeyword())
+ alignment = Left;
+ else if (value == rightKeyword())
+ alignment = Right;
else
es.throwUninformativeAndGenericDOMException(SyntaxError);
@@ -665,17 +686,21 @@ void TextTrackCue::calculateDisplayParameters()
int maximumSize = m_textPosition;
if ((m_writingDirection == Horizontal && m_cueAlignment == Start && m_displayDirection == CSSValueLtr)
|| (m_writingDirection == Horizontal && m_cueAlignment == End && m_displayDirection == CSSValueRtl)
- || (m_writingDirection == VerticalGrowingLeft && m_cueAlignment == Start)
- || (m_writingDirection == VerticalGrowingRight && m_cueAlignment == Start)) {
+ || (m_writingDirection == Horizontal && m_cueAlignment == Left)
+ || (m_writingDirection == VerticalGrowingLeft && (m_cueAlignment == Start || m_cueAlignment == Left))
+ || (m_writingDirection == VerticalGrowingRight && (m_cueAlignment == Start || m_cueAlignment == Left))) {
maximumSize = 100 - m_textPosition;
} else if ((m_writingDirection == Horizontal && m_cueAlignment == End && m_displayDirection == CSSValueLtr)
|| (m_writingDirection == Horizontal && m_cueAlignment == Start && m_displayDirection == CSSValueRtl)
- || (m_writingDirection == VerticalGrowingLeft && m_cueAlignment == End)
- || (m_writingDirection == VerticalGrowingRight && m_cueAlignment == End)) {
+ || (m_writingDirection == Horizontal && m_cueAlignment == Right)
+ || (m_writingDirection == VerticalGrowingLeft && (m_cueAlignment == End || m_cueAlignment == Right))
+ || (m_writingDirection == VerticalGrowingRight && (m_cueAlignment == End || m_cueAlignment == Right))) {
maximumSize = m_textPosition;
} else if (m_cueAlignment == Middle) {
maximumSize = m_textPosition <= 50 ? m_textPosition : (100 - m_textPosition);
maximumSize = maximumSize * 2;
+ } else {
+ ASSERT_NOT_REACHED();
}
// 10.6 If the text track cue size is less than maximum size, then let size
@@ -685,37 +710,62 @@ void TextTrackCue::calculateDisplayParameters()
// 10.8 Determine the value of x-position or y-position for cue as per the
// appropriate rules from the following list:
if (m_writingDirection == Horizontal) {
- if (m_cueAlignment == Start) {
+ switch (m_cueAlignment) {
+ case Start:
if (m_displayDirection == CSSValueLtr)
m_displayPosition.first = m_textPosition;
else
m_displayPosition.first = 100 - m_textPosition - m_displaySize;
- } else if (m_cueAlignment == End) {
+ break;
+ case End:
if (m_displayDirection == CSSValueRtl)
m_displayPosition.first = 100 - m_textPosition;
else
m_displayPosition.first = m_textPosition - m_displaySize;
+ break;
+ case Left:
+ if (m_displayDirection == CSSValueLtr)
+ m_displayPosition.first = m_textPosition;
+ else
+ m_displayPosition.first = 100 - m_textPosition;
+ break;
+ case Right:
+ if (m_displayDirection == CSSValueLtr)
+ m_displayPosition.first = m_textPosition - m_displaySize;
+ else
+ m_displayPosition.first = 100 - m_textPosition - m_displaySize;
+ break;
+ case Middle:
+ if (m_displayDirection == CSSValueLtr)
+ m_displayPosition.first = m_textPosition - m_displaySize / 2;
+ else
+ m_displayPosition.first = 100 - m_textPosition - m_displaySize / 2;
+ break;
+ default:
acolwell GONE FROM CHROMIUM 2013/10/04 17:26:41 nit: You only need default here because of NumberO
philipj_slow 2013/10/07 08:52:32 Isn't it a good idea to catch the case where someo
vcarbune.chromium 2013/10/07 09:37:28 Done.
vcarbune.chromium 2013/10/07 09:37:28 I feel that the current version of the code guards
acolwell GONE FROM CHROMIUM 2013/10/07 15:32:19 We typically guard against this at the assignment
philipj_slow 2013/10/08 06:56:54 Sounds reasonable, I'll keep that in mind when wri
+ ASSERT_NOT_REACHED();
+ }
+ } else {
+ // Cases for m_writingDirection being VerticalGrowing{Left|Right}
+ switch (m_cueAlignment) {
+ case Start:
+ case Left:
+ m_displayPosition.second = m_textPosition;
+ break;
+ case End:
+ case Right:
+ m_displayPosition.second = m_textPosition - m_displaySize;
+ break;
+ case Middle:
+ m_displayPosition.second = m_textPosition - m_displaySize / 2;
+ break;
+ default:
+ ASSERT_NOT_REACHED();
}
}
- if ((m_writingDirection == VerticalGrowingLeft && m_cueAlignment == Start)
- || (m_writingDirection == VerticalGrowingRight && m_cueAlignment == Start)) {
- m_displayPosition.second = m_textPosition;
- } else if ((m_writingDirection == VerticalGrowingLeft && m_cueAlignment == End)
- || (m_writingDirection == VerticalGrowingRight && m_cueAlignment == End)) {
- m_displayPosition.second = 100 - m_textPosition;
- }
-
- if (m_writingDirection == Horizontal && m_cueAlignment == Middle) {
- if (m_displayDirection == CSSValueLtr)
- m_displayPosition.first = m_textPosition - m_displaySize / 2;
- else
- m_displayPosition.first = 100 - m_textPosition - m_displaySize / 2;
- }
-
- if ((m_writingDirection == VerticalGrowingLeft && m_cueAlignment == Middle)
- || (m_writingDirection == VerticalGrowingRight && m_cueAlignment == Middle))
- m_displayPosition.second = m_textPosition - m_displaySize / 2;
+ // A text track cue has a text track cue computed line position whose value
+ // is defined in terms of the other aspects of the cue.
+ m_computedLinePosition = calculateComputedLinePosition();
// 10.9 Determine the value of whichever of x-position or y-position is not
// yet calculated for cue as per the appropriate rules from the following
@@ -732,10 +782,6 @@ void TextTrackCue::calculateDisplayParameters()
if (!m_snapToLines && (m_writingDirection == VerticalGrowingLeft || m_writingDirection == VerticalGrowingRight))
m_displayPosition.first = m_computedLinePosition;
-
- // A text track cue has a text track cue computed line position whose value
- // is defined in terms of the other aspects of the cue.
- m_computedLinePosition = calculateComputedLinePosition();
}
void TextTrackCue::markFutureAndPastNodes(ContainerNode* root, double previousTimestamp, double movieTime)
@@ -1094,6 +1140,14 @@ void TextTrackCue::setCueSettings(const String& input)
// 3. If value is a case-sensitive match for the string "end", then let cue's text track cue alignment be end alignment.
else if (cueAlignment == endKeyword())
m_cueAlignment = End;
+
+ // 4. If value is a case-sensitive match for the string "left", then let cue's text track cue alignment be left alignment.
+ else if (cueAlignment == leftKeyword())
+ m_cueAlignment = Left;
+
+ // 5. If value is a case-sensitive match for the string "right", then let cue's text track cue alignment be right alignment.
+ else if (cueAlignment == rightKeyword())
+ m_cueAlignment = Right;
}
break;
#if ENABLE(WEBVTT_REGIONS)
@@ -1120,6 +1174,11 @@ NextSetting:
#endif
}
+CSSValueID TextTrackCue::getCSSAlignment() const
+{
+ return m_displayAlignmentMap[m_cueAlignment];
+}
+
CSSValueID TextTrackCue::getCSSWritingDirection() const
{
return m_displayDirection;
« Source/core/html/track/TextTrackCue.h ('K') | « Source/core/html/track/TextTrackCue.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698