[WebVTT] Cue with line setting is not rendered correctly
authorpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Oct 2018 18:36:32 +0000 (18:36 +0000)
committerpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Oct 2018 18:36:32 +0000 (18:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190168

Reviewed by Eric Carlson.

Source/WebCore:

When the line setting contains an optional alignment value, the cue is not rendered at the correct position,
see https://w3c.github.io/webvtt/#webvtt-line-cue-setting. This patch does not implement correct handling of
the line setting alignment values, it only makes sure parsing does not fail when the cue has line alignment
settings.

Test: media/track/track-cue-line-position.html

* html/track/VTTCue.cpp:
(WebCore::VTTCueBox::applyCSSProperties):
(WebCore::VTTCue::getPositionCoordinates const):
(WebCore::VTTCue::setCueSettings):

LayoutTests:

* media/track/captions-webvtt/line-position.vtt: Added.
* media/track/track-cue-line-position-expected-mismatch.html: Added.
* media/track/track-cue-line-position.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236754 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/media/track/captions-webvtt/line-position.vtt [new file with mode: 0644]
LayoutTests/media/track/track-cue-line-position-expected-mismatch.html [new file with mode: 0644]
LayoutTests/media/track/track-cue-line-position.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/track/VTTCue.cpp

index 1ef5517..763281f 100644 (file)
@@ -1,3 +1,14 @@
+2018-10-02  Per Arne Vollan  <pvollan@apple.com>
+
+        [WebVTT] Cue with line setting is not rendered correctly
+        https://bugs.webkit.org/show_bug.cgi?id=190168
+
+        Reviewed by Eric Carlson.
+
+        * media/track/captions-webvtt/line-position.vtt: Added.
+        * media/track/track-cue-line-position-expected-mismatch.html: Added.
+        * media/track/track-cue-line-position.html: Added.
+
 2018-10-02  Carlos Eduardo Ramalho  <cadubentzen@gmail.com>
 
         [GTK] fast/forms/color/input-appearance-color.html is failing
diff --git a/LayoutTests/media/track/captions-webvtt/line-position.vtt b/LayoutTests/media/track/captions-webvtt/line-position.vtt
new file mode 100644 (file)
index 0000000..16163d3
--- /dev/null
@@ -0,0 +1,5 @@
+WEBVTT
+
+1
+00:00.000 --> 01:00.000 line:10%,start
+Lorum ipsum
diff --git a/LayoutTests/media/track/track-cue-line-position-expected-mismatch.html b/LayoutTests/media/track/track-cue-line-position-expected-mismatch.html
new file mode 100644 (file)
index 0000000..c6501c4
--- /dev/null
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script>var requirePixelDump = true;</script>
+        <script src=../media-file.js></script>
+        <script src=../video-test.js></script>
+        <script src=../media-controls.js></script>
+
+        <style>
+
+        .obscurer {
+            width: 500px;
+            height: 500px;
+            position: absolute;
+            background-color: white;
+        }
+        </style>
+        <script>
+        function seeked()
+        {
+            endTest();
+        }
+
+        function loaded()
+        {
+            consoleWrite("Test that line position is applied correctly.");
+            findMediaElement();
+            video.src = findMediaFile('video', '../content/test');
+            waitForEvent('seeked', seeked);
+            waitForEvent('canplaythrough', function() { video.currentTime = .5; });
+        }
+
+        setCaptionDisplayMode('Automatic');
+        </script>
+    </head>
+    <body onload="loaded()">
+        <video style="position: absolute; left: 0px; top: 0px;" controls ></video>
+        <div class="obscurer" style="left: 0px; top: 100px;"></div>
+    </body>
+</html>
diff --git a/LayoutTests/media/track/track-cue-line-position.html b/LayoutTests/media/track/track-cue-line-position.html
new file mode 100644 (file)
index 0000000..2ca48cd
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script>var requirePixelDump = true;</script>
+        <script src=../media-file.js></script>
+        <script src=../video-test.js></script>
+        <script src=../media-controls.js></script>
+
+        <style>
+
+        .obscurer {
+            width: 500px;
+            height: 500px;
+            position: absolute;
+            background-color: white;
+        }
+        </style>
+        <script>
+        function seeked()
+        {
+            endTest();
+        }
+
+        function loaded()
+        {
+            consoleWrite("Test that line position is applied correctly.");
+            findMediaElement();
+            video.src = findMediaFile('video', '../content/test');
+            waitForEvent('seeked', seeked);
+            waitForEvent('canplaythrough', function() { video.currentTime = .5; });
+        }
+
+        setCaptionDisplayMode('Automatic');
+        </script>
+    </head>
+    <body onload="loaded()">
+        <video style="position: absolute; left: 0px; top: 0px;" controls >
+            <track src="captions-webvtt/line-position.vtt" kind="captions" default>
+        </video>
+        <div class="obscurer" style="left: 0px; top: 100px;"></div>
+    </body>
+</html>
index e4b3cee..3531f9d 100644 (file)
@@ -1,3 +1,22 @@
+2018-10-02  Per Arne Vollan  <pvollan@apple.com>
+
+        [WebVTT] Cue with line setting is not rendered correctly
+        https://bugs.webkit.org/show_bug.cgi?id=190168
+
+        Reviewed by Eric Carlson.
+
+        When the line setting contains an optional alignment value, the cue is not rendered at the correct position,
+        see https://w3c.github.io/webvtt/#webvtt-line-cue-setting. This patch does not implement correct handling of
+        the line setting alignment values, it only makes sure parsing does not fail when the cue has line alignment
+        settings.
+
+        Test: media/track/track-cue-line-position.html
+
+        * html/track/VTTCue.cpp:
+        (WebCore::VTTCueBox::applyCSSProperties):
+        (WebCore::VTTCue::getPositionCoordinates const):
+        (WebCore::VTTCue::setCueSettings):
+
 2018-10-02  Antti Koivisto  <antti@apple.com>
 
         User installed fonts are not always disabled when they should be
index 9007a4a..9a0d1dd 100644 (file)
@@ -164,13 +164,13 @@ void VTTCueBox::applyCSSProperties(const IntSize& videoSize)
     // the 'writing-mode' property must be set to writing-mode
     setInlineStyleProperty(CSSPropertyWritingMode, m_cue.getCSSWritingMode(), false);
 
-    std::pair<float, float> position = m_cue.getCSSPosition();
+    auto position = m_cue.getCSSPosition();
 
     // the 'top' property must be set to top,
-    setInlineStyleProperty(CSSPropertyTop, static_cast<double>(position.second), CSSPrimitiveValue::CSS_PERCENTAGE);
+    setInlineStyleProperty(CSSPropertyTop, position.second, CSSPrimitiveValue::CSS_PERCENTAGE);
 
     // the 'left' property must be set to left
-    setInlineStyleProperty(CSSPropertyLeft, static_cast<double>(position.first), CSSPrimitiveValue::CSS_PERCENTAGE);
+    setInlineStyleProperty(CSSPropertyLeft, position.first, CSSPrimitiveValue::CSS_PERCENTAGE);
 
     double authorFontSize = std::min(videoSize.width(), videoSize.height()) * DEFAULTCAPTIONFONTSIZEPERCENTAGE / 100.0;
     double multiplier = 1.0;
@@ -925,15 +925,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;
 
+    auto textPosition = calculateComputedTextPosition();
+    
     if (m_writingDirection == Horizontal && m_displayDirection == CSSValueLtr) {
-        coordinates.first = m_textPosition;
+        coordinates.first = textPosition;
         coordinates.second = m_computedLinePosition;
 
         return coordinates;
     }
 
     if (m_writingDirection == Horizontal && m_displayDirection == CSSValueRtl) {
-        coordinates.first = 100 - m_textPosition;
+        coordinates.first = 100 - textPosition;
         coordinates.second = m_computedLinePosition;
 
         return coordinates;
@@ -941,14 +943,14 @@ std::pair<double, double> VTTCue::getPositionCoordinates() const
 
     if (m_writingDirection == VerticalGrowingLeft) {
         coordinates.first = 100 - m_computedLinePosition;
-        coordinates.second = m_textPosition;
+        coordinates.second = textPosition;
 
         return coordinates;
     }
 
     if (m_writingDirection == VerticalGrowingRight) {
         coordinates.first = m_computedLinePosition;
-        coordinates.second = m_textPosition;
+        coordinates.second = textPosition;
 
         return coordinates;
     }
@@ -1039,8 +1041,17 @@ void VTTCue::setCueSettings(const String& inputString)
                     break;
 
                 bool isPercentage = input.scan('%');
-                if (!input.isAt(valueRun.end()))
-                    break;
+                if (!input.isAt(valueRun.end())) {
+                    if (!input.scan(','))
+                        break;
+                    // FIXME: implement handling of line setting alignment.
+                    if (!input.scan(startKeyword().characters8(), startKeyword().length())
+                        && !input.scan(centerKeyword().characters8(), centerKeyword().length())
+                        && !input.scan(endKeyword().characters8(), endKeyword().length())) {
+                        LOG(Media, "VTTCue::setCueSettings, invalid line setting alignment");
+                        break;
+                    }
+                }
 
                 // 2. If value does not contain at least one character in the range U+0030 DIGIT ZERO (0) to U+0039 DIGIT
                 //    NINE (9), then jump to the step labeled next setting.