WebVTT: Support 'auto' text position

Source/core/html/track/vtt/VTTCue.cpp

Index: Source/core/html/track/vtt/VTTCue.cpp
diff --git a/Source/core/html/track/vtt/VTTCue.cpp b/Source/core/html/track/vtt/VTTCue.cpp
index e96b84963494dc62e91620fc17cdfe3ace7e9f28..e5ec5977a049c9d17e6c7f147c59d23d90aceef9 100644
--- a/Source/core/html/track/vtt/VTTCue.cpp
+++ b/Source/core/html/track/vtt/VTTCue.cpp
@@ -32,6 +32,7 @@
 
 #include "bindings/core/v8/ExceptionMessages.h"
 #include "bindings/core/v8/ExceptionStatePlaceholder.h"
+#include "bindings/core/v8/UnionTypesCore.h"
 #include "core/CSSPropertyNames.h"
 #include "core/CSSValueKeywords.h"
 #include "core/dom/DocumentFragment.h"
@@ -46,6 +47,7 @@
 #include "core/html/track/vtt/VTTRegionList.h"
 #include "core/html/track/vtt/VTTScanner.h"
 #include "core/rendering/RenderVTTCue.h"
+#include "platform/FloatConversion.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/text/BidiResolver.h"
 #include "platform/text/TextRunIterator.h"
@@ -54,6 +56,7 @@
 
 namespace blink {
 
+static const float autoPosition = -1;
 static const int undefinedPosition = -1;
 static const int undefinedSize = -1;
 
@@ -69,6 +72,12 @@ static const CSSValueID displayAlignmentMap[] = {
 static_assert(WTF_ARRAY_LENGTH(displayAlignmentMap) == VTTCue::NumberOfAlignments,
     "displayAlignmentMap should have the same number of elements as VTTCue::NumberOfAlignments");
 
+static const String& autoKeyword()
+{
+    DEFINE_STATIC_LOCAL(const String, autoString, ("auto"));
+    return autoString;
+}
+
 static const String& startKeyword()
 {
     DEFINE_STATIC_LOCAL(const String, start, ("start"));
@@ -116,10 +125,11 @@ static const String& verticalGrowingRightKeyword()
     return verticallr;
 }
 
-static bool isInvalidPercentage(int value, ExceptionState& exceptionState)
+template<typename NumberType>
+static bool isInvalidPercentage(NumberType value, ExceptionState& exceptionState)
 {
     if (value < 0 || value > 100) {
-        exceptionState.throwDOMException(IndexSizeError, ExceptionMessages::indexOutsideRange("value", value, 0, ExceptionMessages::InclusiveBound, 100, ExceptionMessages::InclusiveBound));
+        exceptionState.throwDOMException(IndexSizeError, ExceptionMessages::indexOutsideRange<NumberType>("value", value, 0, ExceptionMessages::InclusiveBound, 100, ExceptionMessages::InclusiveBound));
         return true;
     }
     return false;
@@ -211,7 +221,7 @@ VTTCue::VTTCue(Document& document, double startTime, double endTime, const Strin
     , m_text(text)
     , m_linePosition(undefinedPosition)
     , m_computedLinePosition(undefinedPosition)
-    , m_textPosition(50)
+    , m_textPosition(autoPosition)
     , m_cueSize(100)
     , m_writingDirection(Horizontal)
     , m_cueAlignment(Middle)
@@ -324,20 +334,45 @@ void VTTCue::setLine(int position, ExceptionState& exceptionState)
     cueDidChange();
 }
 
-void VTTCue::setPosition(int position, ExceptionState& exceptionState)
+bool VTTCue::textPositionIsAuto() const
 {
-    // http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#dom-texttrackcue-position
-    // On setting, if the new value is negative or greater than 100, then throw an IndexSizeError exception.
-    // Otherwise, set the text track cue text position to the new value.
-    if (isInvalidPercentage(position, exceptionState))
-        return;
+    return m_textPosition == autoPosition;
+}
+
+void VTTCue::position(DoubleOrString& result) const
+{
+    if (textPositionIsAuto())
+        result.setString(autoKeyword());
+    else
+        result.setDouble(m_textPosition);
+}
+
+void VTTCue::setPosition(const DoubleOrString& position, ExceptionState& exceptionState)
+{
+    float specifiedPosition = autoPosition;
+    if (position.isString()) {
+        // FIXME: Would be handled by bindings if the union had AutoKeyword and not DOMString.
+        if (position.getAsString() != autoKeyword())

[philipj_slow, 2015/01/15 09:08:57]

This should throw a TypeError, or whatever cue.align='bah' would throw.

[fs, 2015/01/15 10:07:16]

No, assigning an "invalid" enum value to an attribute does not throw (using it
as an argument does though.)

https://heycam.github.io/webidl/#dfn-enumeration-value (the Note)

[philipj_slow, 2015/01/19 14:44:26]

Acknowledged.

[fs, 2015/01/19 18:10:40]

Actually, now I'm not so sure anymore... [1] (step 4) seems to special case
enumerations ("If the type of the attribute is an enumeration"), but not unions
(with enumerations)...

So:

attribute Foo bar; // Does not throw
attribute (double or Foo) baz; // Throws

I think I'll just go with whatever the impl. thinks is god for now...

[1] http://heycam.github.io/webidl/#dfn-attribute-setter

+            return;
+    } else {
+        ASSERT(position.isDouble());
+        double doublePosition = position.getAsDouble();
+
+        // http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#dom-texttrackcue-position

[philipj_slow, 2015/01/15 09:08:57]

Broken link, should be http://dev.w3.org/html5/webvtt/#dfn-vttcue-position
Maybe put the spec text at the top of the function?

+        // On setting, if the new value is negative or greater than 100, then throw an IndexSizeError exception.
+        // Otherwise, set the text track cue text position to the new value.
+        if (isInvalidPercentage(doublePosition, exceptionState))
+            return;
+
+        specifiedPosition = narrowPrecisionToFloat(doublePosition);
+    }
 
     // Otherwise, set the text track cue line position to the new value.
-    if (m_textPosition == position)
+    if (m_textPosition == specifiedPosition)
         return;
 
     cueWillChange();
-    m_textPosition = position;
+    m_textPosition = specifiedPosition;
     cueDidChange();
 }
 
@@ -554,6 +589,34 @@ static CSSValueID determineTextDirection(DocumentFragment* vttRoot)
     return isLeftToRightDirection(textDirection) ? CSSValueLtr : CSSValueRtl;
 }
 
+int VTTCue::calculateComputedTextPosition() const
+{
+    // http://dev.w3.org/html5/webvtt/#dfn-text-track-cue-computed-text-position
+
+    // 1. If the text track cue text position is numeric, then return the value
+    // of the text track cue text position and abort these steps. (Otherwise,
+    // the text track cue text position is the special value auto.)
+    if (!textPositionIsAuto())
+        return static_cast<int>(m_textPosition);

[philipj_slow, 2015/01/15 09:08:57]

Per spec there is no rounding of position to integer, just line in the
snap-to-lines case. I think it would be best to let the floats flow all the way
in this change, people would be justified in assuming that
'(cue.position=0.5)==0.5' implies that the layout also gets the float.

[fs, 2015/01/15 10:07:16]

I was trying to avoid "two steps at once" here by keeping the layout change as
small as possible. With the split, that's probably less of an issue though.

[philipj_slow, 2015/01/19 14:44:26]

As long as the whole change ends up in the same milestone so that the feature
detection works I'm OK with splitting across CLs as you see fit.

[fs, 2015/01/19 15:09:05]

Yes, that should be feasible. I'm currently looking at landing this as:

1) long->double
2) Update VTTCue.line
3) Update VTTCue.position

So still 2 CLs to go before returning to this one.

+
+    switch (m_cueAlignment) {
+    // 2. If the text track cue text alignment is start or left, return 0 and abort these steps.
+    case Start:
+    case Left:
+        return 0;
+    // 3. If the text track cue text alignment is end or right, return 100 and abort these steps.
+    case End:
+    case Right:
+        return 100;
+    // 4. If the text track cue text alignment is middle, return 50 and abort these steps.
+    case Middle:
+        return 50;
+    default:
+        ASSERT_NOT_REACHED();
+        return 0;
+    }
+}
+
 void VTTCue::calculateDisplayParameters()
 {
     createVTTNodeTree();
@@ -574,21 +637,22 @@ void VTTCue::calculateDisplayParameters()
 
     // 10.5 Determine the value of maximum size for cue as per the appropriate
     // rules from the following list:
-    int maximumSize = m_textPosition;
+    int computedTextPosition = calculateComputedTextPosition();
+    int maximumSize = computedTextPosition;
     if ((m_writingDirection == Horizontal && m_cueAlignment == Start && m_displayDirection == CSSValueLtr)
         || (m_writingDirection == Horizontal && m_cueAlignment == End && m_displayDirection == CSSValueRtl)
         || (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;
+        maximumSize = 100 - computedTextPosition;
     } else if ((m_writingDirection == Horizontal && m_cueAlignment == End && m_displayDirection == CSSValueLtr)
         || (m_writingDirection == Horizontal && m_cueAlignment == Start && m_displayDirection == CSSValueRtl)
         || (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;
+        maximumSize = computedTextPosition;
     } else if (m_cueAlignment == Middle) {
-        maximumSize = m_textPosition <= 50 ? m_textPosition : (100 - m_textPosition);
+        maximumSize = computedTextPosition <= 50 ? computedTextPosition : (100 - computedTextPosition);
         maximumSize = maximumSize * 2;
     } else {
         ASSERT_NOT_REACHED();
@@ -607,33 +671,33 @@ void VTTCue::calculateDisplayParameters()
         switch (m_cueAlignment) {
         case Start:
             if (m_displayDirection == CSSValueLtr)
-                m_displayPosition.first = m_textPosition;
+                m_displayPosition.first = computedTextPosition;
             else
-                m_displayPosition.first = 100 - m_textPosition - m_displaySize;
+                m_displayPosition.first = 100 - computedTextPosition - m_displaySize;
             break;
         case End:
             if (m_displayDirection == CSSValueRtl)
-                m_displayPosition.first = 100 - m_textPosition;
+                m_displayPosition.first = 100 - computedTextPosition;
             else
-                m_displayPosition.first = m_textPosition - m_displaySize;
+                m_displayPosition.first = computedTextPosition - m_displaySize;
             break;
         case Left:
             if (m_displayDirection == CSSValueLtr)
-                m_displayPosition.first = m_textPosition;
+                m_displayPosition.first = computedTextPosition;
             else
-                m_displayPosition.first = 100 - m_textPosition;
+                m_displayPosition.first = 100 - computedTextPosition;
             break;
         case Right:
             if (m_displayDirection == CSSValueLtr)
-                m_displayPosition.first = m_textPosition - m_displaySize;
+                m_displayPosition.first = computedTextPosition - m_displaySize;
             else
-                m_displayPosition.first = 100 - m_textPosition - m_displaySize;
+                m_displayPosition.first = 100 - computedTextPosition - m_displaySize;
             break;
         case Middle:
             if (m_displayDirection == CSSValueLtr)
-                m_displayPosition.first = m_textPosition - m_displaySize / 2;
+                m_displayPosition.first = computedTextPosition - m_displaySize / 2;
             else
-                m_displayPosition.first = 100 - m_textPosition - m_displaySize / 2;
+                m_displayPosition.first = 100 - computedTextPosition - m_displaySize / 2;
             break;
         default:
             ASSERT_NOT_REACHED();
@@ -643,14 +707,14 @@ void VTTCue::calculateDisplayParameters()
         switch (m_cueAlignment) {
         case Start:
         case Left:
-            m_displayPosition.second = m_textPosition;
+            m_displayPosition.second = computedTextPosition;
             break;
         case End:
         case Right:
-            m_displayPosition.second = m_textPosition - m_displaySize;
+            m_displayPosition.second = computedTextPosition - m_displaySize;
             break;
         case Middle:
-            m_displayPosition.second = m_textPosition - m_displaySize / 2;
+            m_displayPosition.second = computedTextPosition - m_displaySize / 2;
             break;
         default:
             ASSERT_NOT_REACHED();
@@ -793,7 +857,7 @@ void VTTCue::updateDisplay(const IntSize& videoSize, HTMLDivElement& container)
     if (m_linePosition != undefinedPosition)
         UseCounter::count(document(), UseCounter::VTTCueRenderLineNotAuto);
 
-    if (m_textPosition != 50)
+    if (textPositionIsAuto())

[fs, 2015/01/14 13:13:40]

Changed this on the assumption that we want to check the default value (which
means this is slightly more narrow than the previous one.)

[philipj_slow, 2015/01/15 09:08:57]

Yeah. Too bad the name will suck, but oh well.

         UseCounter::count(document(), UseCounter::VTTCueRenderPositionNot50);
 
     if (m_cueSize != 100)
@@ -832,16 +896,17 @@ std::pair<double, double> VTTCue::getPositionCoordinates() const
 {
     // This method is used for setting x and y when snap to lines is not set.
     std::pair<double, double> coordinates;
+    int computedTextPosition = calculateComputedTextPosition();
 
     if (m_writingDirection == Horizontal && m_displayDirection == CSSValueLtr) {
-        coordinates.first = m_textPosition;
+        coordinates.first = computedTextPosition;
         coordinates.second = m_computedLinePosition;
 
         return coordinates;
     }
 
     if (m_writingDirection == Horizontal && m_displayDirection == CSSValueRtl) {
-        coordinates.first = 100 - m_textPosition;
+        coordinates.first = 100 - computedTextPosition;
         coordinates.second = m_computedLinePosition;
 
         return coordinates;
@@ -849,14 +914,14 @@ std::pair<double, double> VTTCue::getPositionCoordinates() const
 
     if (m_writingDirection == VerticalGrowingLeft) {
         coordinates.first = 100 - m_computedLinePosition;
-        coordinates.second = m_textPosition;
+        coordinates.second = computedTextPosition;
 
         return coordinates;
     }
 
     if (m_writingDirection == VerticalGrowingRight) {
         coordinates.first = m_computedLinePosition;
-        coordinates.second = m_textPosition;
+        coordinates.second = computedTextPosition;
 
         return coordinates;
     }