Determine text direction for rendering a TextTrackCue
authorvcarbune@chromium.org <vcarbune@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Mar 2013 22:02:45 +0000 (22:02 +0000)
committervcarbune@chromium.org <vcarbune@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Mar 2013 22:02:45 +0000 (22:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79749

Reviewed by Levi Weintraub.

Source/WebCore:

The rendering rules for WebVTT cues are slightly different
depending on the directionality determined from the characters
of the cue text. This patch implements the preliminary step
to be able to take directionality into account.

It iterates through the cue characters and setting the CSS
directionality to the one of the first strong directional character.

Test: media/track/track-cue-rendering-rtl.html

* html/track/TextTrackCue.cpp:
(WebCore::TextTrackCueBox::applyCSSProperties): Applies the determined
direction.
(WebCore::TextTrackCue::TextTrackCue): Sets the default value for the
display direction.
(WebCore::isCueParagraphSeparator): Tests for explicit unicode characters
that are paragraph separators.
(WebCore):
(WebCore::TextTrackCue::determineTextDirection): Determined the direction
by the first strong directional character found.
(WebCore::TextTrackCue::calculateDisplayParameters): Updated.
(WebCore::TextTrackCue::getCSSWritingDirection): Return the determined CSS
writing direction.
* html/track/TextTrackCue.h:
(TextTrackCue):

LayoutTests:

* media/track/captions-webvtt/captions-rtl.vtt: Added.
* media/track/track-cue-rendering-rtl-expected.txt: Added.
* media/track/track-cue-rendering-rtl.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/media/track/captions-webvtt/captions-rtl.vtt [new file with mode: 0644]
LayoutTests/media/track/track-cue-rendering-rtl-expected.txt [new file with mode: 0644]
LayoutTests/media/track/track-cue-rendering-rtl.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/track/TextTrackCue.cpp
Source/WebCore/html/track/TextTrackCue.h

index fa6f351..3d7f433 100644 (file)
@@ -1,3 +1,14 @@
+2013-03-18  Victor Carbune  <vcarbune@chromium.org>
+
+        Determine text direction for rendering a TextTrackCue
+        https://bugs.webkit.org/show_bug.cgi?id=79749
+
+        Reviewed by Levi Weintraub.
+
+        * media/track/captions-webvtt/captions-rtl.vtt: Added.
+        * media/track/track-cue-rendering-rtl-expected.txt: Added.
+        * media/track/track-cue-rendering-rtl.html: Added.
+
 2013-03-18  Kenneth Russell  <kbr@google.com>
 
         Unreviewed gardening. Marked flaky tests, removed obsolete
diff --git a/LayoutTests/media/track/captions-webvtt/captions-rtl.vtt b/LayoutTests/media/track/captions-webvtt/captions-rtl.vtt
new file mode 100644 (file)
index 0000000..06185bd
--- /dev/null
@@ -0,0 +1,27 @@
+WEBVTT
+Examples of RTL (odd-numbered ids) and LTR cues (even-numbered ids).
+
+1
+00:00:00.000 --> 00:00:00.500
+تجربة
+
+2
+00:00:00.500 --> 00:00:01.000
+       1234
+
+3
+00:00:01.000 --> 00:00:01.500
+تجربة
+LTR new line, but cue should be RTL
+
+4
+00:00:01.500 --> 00:00:02.000
+LTR cue تجربة
+
+5
+00:00:02.000 --> 00:00:02.500
+;1234تجربة
+
+6
+00:00:02.500 --> 00:00:03.000
+       ०१२३४५६७८९          
diff --git a/LayoutTests/media/track/track-cue-rendering-rtl-expected.txt b/LayoutTests/media/track/track-cue-rendering-rtl-expected.txt
new file mode 100644 (file)
index 0000000..f528594
--- /dev/null
@@ -0,0 +1,82 @@
+Test that directionality is set correctly on cues.
+
+** RTL cues alternate with LTR cues **
+EVENT(canplaythrough)
+EVENT(seeked)
+
+** Jump to next cue **
+EXPECTED (video.currentTime == '0.25') OK
+EXPECTED (testTrack.track.activeCues.length == '1') OK
+EXPECTED (testTrack.track.activeCues[0].text == 'تجربة') OK
+EXPECTED (testCueDisplayBox.innerText == 'تجربة') OK
+
+** The position should be default and CSS direction set to rtl **
+EXPECTED (2 * testCueDisplayBox.offsetLeft == video.videoWidth - testCueDisplayBox.offsetWidth == 'true') OK
+EXPECTED (getComputedStyle(testCueDisplayBox).direction == 'rtl') OK
+
+RUN(video.currentTime = 0.75)
+EVENT(seeked)
+
+** Jump to next cue **
+EXPECTED (video.currentTime == '0.75') OK
+EXPECTED (testTrack.track.activeCues.length == '1') OK
+EXPECTED (testTrack.track.activeCues[0].text == '      1234') OK
+EXPECTED (testCueDisplayBox.innerText == '     1234') OK
+
+** The position should be default and CSS direction set to ltr **
+EXPECTED (2 * testCueDisplayBox.offsetLeft == video.videoWidth - testCueDisplayBox.offsetWidth == 'true') OK
+EXPECTED (getComputedStyle(testCueDisplayBox).direction == 'ltr') OK
+
+RUN(video.currentTime = 1.25)
+EVENT(seeked)
+
+** Jump to next cue **
+EXPECTED (video.currentTime == '1.25') OK
+EXPECTED (testTrack.track.activeCues.length == '1') OK
+EXPECTED (testTrack.track.activeCues[0].text == 'تجربة LTR new line, but cue should be RTL') OK
+EXPECTED (testCueDisplayBox.innerText == 'تجربة LTR new line, but cue should be RTL') OK
+
+** The position should be default and CSS direction set to rtl **
+EXPECTED (2 * testCueDisplayBox.offsetLeft == video.videoWidth - testCueDisplayBox.offsetWidth == 'true') OK
+EXPECTED (getComputedStyle(testCueDisplayBox).direction == 'rtl') OK
+
+RUN(video.currentTime = 1.75)
+EVENT(seeked)
+
+** Jump to next cue **
+EXPECTED (video.currentTime == '1.75') OK
+EXPECTED (testTrack.track.activeCues.length == '1') OK
+EXPECTED (testTrack.track.activeCues[0].text == 'LTR cue تجربة') OK
+EXPECTED (testCueDisplayBox.innerText == 'LTR cue تجربة') OK
+
+** The position should be default and CSS direction set to ltr **
+EXPECTED (2 * testCueDisplayBox.offsetLeft == video.videoWidth - testCueDisplayBox.offsetWidth == 'true') OK
+EXPECTED (getComputedStyle(testCueDisplayBox).direction == 'ltr') OK
+
+RUN(video.currentTime = 2.25)
+EVENT(seeked)
+
+** Jump to next cue **
+EXPECTED (video.currentTime == '2.25') OK
+EXPECTED (testTrack.track.activeCues.length == '1') OK
+EXPECTED (testTrack.track.activeCues[0].text == ';1234تجربة') OK
+EXPECTED (testCueDisplayBox.innerText == ';1234تجربة') OK
+
+** The position should be default and CSS direction set to rtl **
+EXPECTED (2 * testCueDisplayBox.offsetLeft == video.videoWidth - testCueDisplayBox.offsetWidth == 'true') OK
+EXPECTED (getComputedStyle(testCueDisplayBox).direction == 'rtl') OK
+
+RUN(video.currentTime = 2.75)
+EVENT(seeked)
+
+** Jump to next cue **
+EXPECTED (video.currentTime == '2.75') OK
+EXPECTED (testTrack.track.activeCues.length == '1') OK
+EXPECTED (testTrack.track.activeCues[0].text == '      ०१२३४५६७८९ ') OK
+EXPECTED (testCueDisplayBox.innerText == '     ०१२३४५६७८९ ') OK
+
+** The position should be default and CSS direction set to ltr **
+EXPECTED (2 * testCueDisplayBox.offsetLeft == video.videoWidth - testCueDisplayBox.offsetWidth == 'true') OK
+EXPECTED (getComputedStyle(testCueDisplayBox).direction == 'ltr') OK
+END OF TEST
+
diff --git a/LayoutTests/media/track/track-cue-rendering-rtl.html b/LayoutTests/media/track/track-cue-rendering-rtl.html
new file mode 100644 (file)
index 0000000..a4a66ef
--- /dev/null
@@ -0,0 +1,77 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+
+        <script src=../media-file.js></script>
+        <script src=../video-test.js></script>
+        <script src=../media-controls.js></script>
+
+        <script>
+
+        var testTrack;
+        var testCueDisplayBox;
+        var seekedCount = 0;
+        var direction;
+
+        var info = ["تجربة",
+                    "\t1234",
+                    "تجربة\nLTR new line, but cue should be RTL",
+                    "LTR cue تجربة",
+                    ";1234تجربة",
+                    "\t०१२३४५६७८९  \t"];
+
+        function testCueStyle()
+        {
+            endTest();
+        }
+
+        function seeked()
+        {
+            testCueDisplayBox = textTrackDisplayElement(video, 'display');
+
+            consoleWrite("");
+            consoleWrite("** Jump to next cue **");
+            testExpected("video.currentTime", (seekedCount / 2) + .25);
+            testExpected("testTrack.track.activeCues.length", 1);
+            testExpected("testTrack.track.activeCues[0].text", info[seekedCount]);
+            testExpected("testCueDisplayBox.innerText", info[seekedCount]);
+
+            direction = seekedCount % 2 ? "ltr" : "rtl";
+
+            consoleWrite("");
+            consoleWrite("** The position should be default and CSS direction set to " + direction + " **");
+            testExpected("2 * testCueDisplayBox.offsetLeft == video.videoWidth - testCueDisplayBox.offsetWidth", true);
+
+            testExpected("getComputedStyle(testCueDisplayBox).direction", direction);
+
+            if (++seekedCount == info.length)
+                endTest();
+            else {
+                consoleWrite("");
+                run("video.currentTime = " + (video.currentTime + .5));
+                return;
+            }
+        }
+
+        function loaded()
+        {
+            consoleWrite("Test that directionality is set correctly on cues.");
+            testTrack = document.querySelector('track');
+
+            findMediaElement();
+            video.src = findMediaFile('video', '../content/test');
+
+            consoleWrite("");
+            consoleWrite("** RTL cues alternate with LTR cues **");
+            waitForEvent('seeked', seeked);
+            waitForEvent('canplaythrough', function() { video.currentTime = .25; });
+        }
+        </script>
+    </head>
+    <body onload="loaded()">
+        <video controls >
+            <track src="captions-webvtt/captions-rtl.vtt" kind="captions" default>
+        </video>
+    </body>
+</html>
index 702acb9..0f6a0da 100644 (file)
@@ -1,3 +1,36 @@
+2013-03-18  Victor Carbune  <vcarbune@chromium.org>
+
+        Determine text direction for rendering a TextTrackCue
+        https://bugs.webkit.org/show_bug.cgi?id=79749
+
+        Reviewed by Levi Weintraub.
+
+        The rendering rules for WebVTT cues are slightly different
+        depending on the directionality determined from the characters
+        of the cue text. This patch implements the preliminary step
+        to be able to take directionality into account.
+
+        It iterates through the cue characters and setting the CSS
+        directionality to the one of the first strong directional character.
+
+        Test: media/track/track-cue-rendering-rtl.html
+
+        * html/track/TextTrackCue.cpp:
+        (WebCore::TextTrackCueBox::applyCSSProperties): Applies the determined
+        direction.
+        (WebCore::TextTrackCue::TextTrackCue): Sets the default value for the
+        display direction.
+        (WebCore::isCueParagraphSeparator): Tests for explicit unicode characters
+        that are paragraph separators.
+        (WebCore):
+        (WebCore::TextTrackCue::determineTextDirection): Determined the direction
+        by the first strong directional character found.
+        (WebCore::TextTrackCue::calculateDisplayParameters): Updated.
+        (WebCore::TextTrackCue::getCSSWritingDirection): Return the determined CSS
+        writing direction.
+        * html/track/TextTrackCue.h:
+        (TextTrackCue):
+
 2013-03-18  Hans Wennborg  <hans@chromium.org>
 
         Fix GridTrackSize::operator==
index 5ee3d22..3d3ea8f 100644 (file)
@@ -119,9 +119,8 @@ void TextTrackCueBox::applyCSSProperties(const IntSize&)
     //  the 'unicode-bidi' property must be set to 'plaintext'
     setInlineStyleProperty(CSSPropertyUnicodeBidi, CSSValueWebkitPlaintext);
 
-    // FIXME: Determine the text direction using the BIDI algorithm. http://wkb.ug/79749
     // the 'direction' property must be set to direction
-    setInlineStyleProperty(CSSPropertyDirection, CSSValueLtr);
+    setInlineStyleProperty(CSSPropertyDirection, m_cue->getCSSWritingDirection());
 
     // the 'writing-mode' property must be set to writing-mode
     setInlineStyleProperty(CSSPropertyWebkitWritingMode, m_cue->getCSSWritingMode(), false);
@@ -204,6 +203,7 @@ TextTrackCue::TextTrackCue(ScriptExecutionContext* context, double start, double
     , m_snapToLines(true)
     , m_cueBackgroundBox(HTMLDivElement::create(toDocument(context)))
     , m_displayTreeShouldChange(true)
+    , m_displayDirection(CSSValueLtr)
 {
     ASSERT(m_scriptExecutionContext->isDocument());
 
@@ -584,11 +584,58 @@ int TextTrackCue::calculateComputedLinePosition()
     return n;
 }
 
+static bool isCueParagraphSeparator(UChar character)
+{
+    // Within a cue, paragraph boundaries are only denoted by Type B characters,
+    // such as U+000A LINE FEED (LF), U+0085 NEXT LINE (NEL), and U+2029 PARAGRAPH SEPARATOR.
+    return WTF::Unicode::category(character) & WTF::Unicode::Separator_Paragraph;
+}
+
+void TextTrackCue::determineTextDirection()
+{
+    DEFINE_STATIC_LOCAL(const String, rtTag, (ASCIILiteral("rt")));
+    createWebVTTNodeTree();
+
+    // Apply the Unicode Bidirectional Algorithm's Paragraph Level steps to the
+    // concatenation of the values of each WebVTT Text Object in nodes, in a
+    // pre-order, depth-first traversal, excluding WebVTT Ruby Text Objects and
+    // their descendants.
+    StringBuilder paragraphBuilder;
+    for (Node* node = m_webVTTNodeTree->firstChild(); node; node = NodeTraversal::next(node, m_webVTTNodeTree.get())) {
+        if (!node->isTextNode() || node->localName() == rtTag)
+            continue;
+
+        paragraphBuilder.append(node->nodeValue());
+    }
+
+    String paragraph = paragraphBuilder.toString();
+    if (!paragraph.length())
+        return;
+
+    for (size_t i = 0; i < paragraph.length(); ++i) {
+        UChar current = paragraph[i];
+        if (!current || isCueParagraphSeparator(current))
+            return;
+
+        if (UChar current = paragraph[i]) {
+            WTF::Unicode::Direction charDirection = WTF::Unicode::direction(current);
+            if (charDirection == WTF::Unicode::LeftToRight) {
+                m_displayDirection = CSSValueLtr;
+                return;
+            }
+            if (charDirection == WTF::Unicode::RightToLeft
+                || charDirection == WTF::Unicode::RightToLeftArabic) {
+                m_displayDirection = CSSValueRtl;
+                return;
+            }
+        }
+    }
+}
+
 void TextTrackCue::calculateDisplayParameters()
 {
-    // FIXME(BUG 79749): Determine the text direction using the BIDI algorithm.
     // Steps 10.2, 10.3
-    m_displayDirection = CSSValueLtr;
+    determineTextDirection();
 
     // 10.4 If the text track cue writing direction is horizontal, then let
     // block-flow be 'tb'. Otherwise, if the text track cue writing direction is
@@ -1028,6 +1075,11 @@ NextSetting:
     }
 }
 
+int TextTrackCue::getCSSWritingDirection() const
+{
+    return m_displayDirection;
+}
+
 int TextTrackCue::getCSSWritingMode() const
 {
     return m_displayWritingMode;
index b5c5cf4..4808b19 100644 (file)
@@ -151,7 +151,9 @@ public:
     virtual ScriptExecutionContext* scriptExecutionContext() const;
 
     std::pair<double, double> getCSSPosition() const;
+
     int getCSSSize() const;
+    int getCSSWritingDirection() const;
     int getCSSWritingMode() const;
 
     enum WritingDirection {
@@ -202,6 +204,7 @@ private:
     std::pair<double, double> getPositionCoordinates() const;
     void parseSettings(const String&);
 
+    void determineTextDirection();
     void calculateDisplayParameters();
 
     void cueWillChange();