Title: [221353] releases/WebKitGTK/webkit-2.18
Revision
221353
Author
carlo...@webkit.org
Date
2017-08-30 02:14:49 -0700 (Wed, 30 Aug 2017)

Log Message

Merge r221155 - HTMLTrackElement behavior violates the standard
https://bugs.webkit.org/show_bug.cgi?id=175888

Patch by Kirill Ovchinnikov <kirill.ovch...@gmail.com> on 2017-08-24
Reviewed by Eric Carlson.

Source/WebCore:

Test: media/track/text-track-src-change.html: added asserts

* html/HTMLTrackElement.cpp:
(WebCore::HTMLTrackElement::parseAttribute):
(WebCore::HTMLTrackElement::loadTimerFired):
* html/track/LoadableTextTrack.cpp:
(WebCore::LoadableTextTrack::scheduleLoad):
* html/track/TextTrack.cpp:
(WebCore::TextTrack::removeAllCues):
* html/track/TextTrackCueList.cpp:
(WebCore::TextTrackCueList::removeAll):
* html/track/TextTrackCueList.h:

LayoutTests:

* media/track/text-track-src-change-expected.txt:
* media/track/text-track-src-change.html:

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.18/LayoutTests/ChangeLog (221352 => 221353)


--- releases/WebKitGTK/webkit-2.18/LayoutTests/ChangeLog	2017-08-30 09:00:12 UTC (rev 221352)
+++ releases/WebKitGTK/webkit-2.18/LayoutTests/ChangeLog	2017-08-30 09:14:49 UTC (rev 221353)
@@ -1,3 +1,13 @@
+2017-08-24  Kirill Ovchinnikov  <kirill.ovch...@gmail.com>
+
+        HTMLTrackElement behavior violates the standard
+        https://bugs.webkit.org/show_bug.cgi?id=175888
+
+        Reviewed by Eric Carlson.
+
+        * media/track/text-track-src-change-expected.txt:
+        * media/track/text-track-src-change.html:
+
 2017-08-23  Wenson Hsieh  <wenson_hs...@apple.com>
 
         DeleteSelectionCommand should be robust when starting and ending editable positions cannot be found

Modified: releases/WebKitGTK/webkit-2.18/LayoutTests/media/track/text-track-src-change-expected.txt (221352 => 221353)


--- releases/WebKitGTK/webkit-2.18/LayoutTests/media/track/text-track-src-change-expected.txt	2017-08-30 09:00:12 UTC (rev 221352)
+++ releases/WebKitGTK/webkit-2.18/LayoutTests/media/track/text-track-src-change-expected.txt	2017-08-30 09:14:49 UTC (rev 221353)
@@ -7,8 +7,19 @@
 
 *** Changing 'src' attribute...
 EXPECTED (cues.length == '100') OK
+EXPECTED (cues === testTrack.track.cues == 'true') OK
 
 *** Changing back 'src' attribute...
+EXPECTED (cues === testTrack.track.cues == 'true') OK
 EXPECTED (cues.length == '4') OK
+
+*** Setting 'src' to the same value...
+EXPECTED (cues === testTrack.track.cues == 'true') OK
+EXPECTED (cues.length == '4') OK
+
+*** Setting 'src' to an empty value...
+EXPECTED (stage == '5') OK
+EXPECTED (true == 'true') OK
+EXPECTED (cues.length == '0') OK
 END OF TEST
 

Modified: releases/WebKitGTK/webkit-2.18/LayoutTests/media/track/text-track-src-change.html (221352 => 221353)


--- releases/WebKitGTK/webkit-2.18/LayoutTests/media/track/text-track-src-change.html	2017-08-30 09:00:12 UTC (rev 221352)
+++ releases/WebKitGTK/webkit-2.18/LayoutTests/media/track/text-track-src-change.html	2017-08-30 09:14:49 UTC (rev 221353)
@@ -7,8 +7,9 @@
         <script src=""
         <script>
             var stage = 0; 
+            var errorEventTimer = null;
 
-            function trackLoaded()
+            function loadedTrackTest()
             {
                 var testTrack = document.getElementById('testTrack');
                 switch (stage) {
@@ -21,20 +22,51 @@
                         testTrack.src = ""
                         break;
                     case 1:
-                        cues = testTrack.track.cues;
                         testExpected("cues.length", 100);
+                        testExpected("cues === testTrack.track.cues", true);
                         consoleWrite("<br>*** Changing back 'src' attribute...");
                         ++stage;
                         testTrack.src = ""
                         break;
                     case 2:
-                        cues = testTrack.track.cues;
+                        testExpected("cues === testTrack.track.cues", true);
                         testExpected("cues.length", 4);
-                        endTest();
+                        consoleWrite("<br>*** Setting 'src' to the same value...");
+                        ++stage;
+                        testTrack.src = ""
+                        // Let's ensure that 'load' event is not fired after that
+                        // - it should jump directly to the stage 4 instead of stage 3
+                        setTimeout(function() { ++stage; loadedTrackTest(); }, 100);
                         break;
+                    case 3:
+                        failTest("'load' event should not be fired after setting the same URL");
+                        break;
+                    case 4:
+                        testExpected("cues === testTrack.track.cues", true);
+                        testExpected("cues.length", 4);
+                        consoleWrite("<br>*** Setting 'src' to an empty value...");
+                        ++stage;
+                        testTrack.src = "" // this should fire 'error' event
+                        errorEventTimer = setTimeout(function() {
+                                failTest("'error' event didn't fire after setting an empty URL");
+                            }, 10000);
+                        break;
+                    case 5:
+                        failTest("'load' event should not be fired after setting an empty URL");
+                        break;
                 }
             }
 
+            function trackLoadFailed()
+            {
+                clearTimeout(errorEventTimer);
+                testExpected("stage", 5);
+                testExpected(cues === testTrack.track.cues, true);
+                testExpected("cues.length", 0);
+                endTest();
+            }
+
+
             setCaptionDisplayMode('Automatic');
         </script>
     </head>
@@ -41,7 +73,7 @@
     <body>
         <p>Tests Track 'src' changing handling</p>
         <video>
-            <track id="testTrack" src="" kind="captions" _onload_="trackLoaded()" default>
+            <track id="testTrack" src="" kind="captions" _onload_="loadedTrackTest()" _onerror_="trackLoadFailed()" default>
         </video>
     </body>
 </html>

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog (221352 => 221353)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2017-08-30 09:00:12 UTC (rev 221352)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2017-08-30 09:14:49 UTC (rev 221353)
@@ -1,3 +1,23 @@
+2017-08-24  Kirill Ovchinnikov  <kirill.ovch...@gmail.com>
+
+        HTMLTrackElement behavior violates the standard
+        https://bugs.webkit.org/show_bug.cgi?id=175888
+
+        Reviewed by Eric Carlson.
+
+        Test: media/track/text-track-src-change.html: added asserts
+
+        * html/HTMLTrackElement.cpp:
+        (WebCore::HTMLTrackElement::parseAttribute):
+        (WebCore::HTMLTrackElement::loadTimerFired):
+        * html/track/LoadableTextTrack.cpp:
+        (WebCore::LoadableTextTrack::scheduleLoad):
+        * html/track/TextTrack.cpp:
+        (WebCore::TextTrack::removeAllCues):
+        * html/track/TextTrackCueList.cpp:
+        (WebCore::TextTrackCueList::removeAll):
+        * html/track/TextTrackCueList.h:
+
 2017-08-23  Adrian Perez de Castro  <ape...@igalia.com>
 
         Geoclue2 based backend should provide the right desktop ID

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/html/HTMLTrackElement.cpp (221352 => 221353)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/html/HTMLTrackElement.cpp	2017-08-30 09:00:12 UTC (rev 221352)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/html/HTMLTrackElement.cpp	2017-08-30 09:14:49 UTC (rev 221353)
@@ -100,10 +100,7 @@
 void HTMLTrackElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
     if (name == srcAttr) {
-        if (!value.isEmpty())
-            scheduleLoad();
-        else if (m_track)
-            m_track->removeAllCues();
+        scheduleLoad();
 
     // 4.8.10.12.3 Sourcing out-of-band text tracks
     // As the kind, label, and srclang attributes are set, changed, or removed, the text track must update accordingly...
@@ -186,8 +183,10 @@
 
 void HTMLTrackElement::loadTimerFired()
 {
-    if (!hasAttributeWithoutSynchronization(srcAttr))
+    if (!hasAttributeWithoutSynchronization(srcAttr)) {
+        track().removeAllCues();
         return;
+    }
 
     // 6. Set the text track readiness state to loading.
     setReadyState(HTMLTrackElement::LOADING);
@@ -195,17 +194,16 @@
     // 7. Let URL be the track URL of the track element.
     URL url = ""
 
+    // ... if URL is the empty string, then queue a task to first change the text track readiness state
+    // to failed to load and then fire an event named error at the track element.
     // 8. If the track element's parent is a media element then let CORS mode be the state of the parent media
     // element's crossorigin content attribute. Otherwise, let CORS mode be No CORS.
-    if (!canLoadURL(url)) {
+    if (url.isEmpty() || !canLoadURL(url)) {
+        track().removeAllCues();
         didCompleteLoad(HTMLTrackElement::Failure);
         return;
     }
 
-    // When src attribute is changed we need to flush all collected track data
-    if (m_track)
-        m_track->removeAllCues();
-
     track().scheduleLoad(url);
 }
 

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/LoadableTextTrack.cpp (221352 => 221353)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/LoadableTextTrack.cpp	2017-08-30 09:00:12 UTC (rev 221352)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/LoadableTextTrack.cpp	2017-08-30 09:14:49 UTC (rev 221353)
@@ -48,6 +48,9 @@
     if (url == m_url)
         return;
 
+    // When src attribute is changed we need to flush all collected track data
+    removeAllCues();
+
     // 4.8.10.12.3 Sourcing out-of-band text tracks (continued)
 
     // 2. Let URL be the track URL of the track element.

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/TextTrack.cpp (221352 => 221353)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/TextTrack.cpp	2017-08-30 09:00:12 UTC (rev 221352)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/TextTrack.cpp	2017-08-30 09:14:49 UTC (rev 221353)
@@ -269,7 +269,7 @@
     for (size_t i = 0; i < m_cues->length(); ++i)
         m_cues->item(i)->setTrack(nullptr);
     
-    m_cues = nullptr;
+    m_cues->clear();
 }
 
 TextTrackCueList* TextTrack::activeCues() const

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/TextTrackCueList.cpp (221352 => 221353)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/TextTrackCueList.cpp	2017-08-30 09:00:12 UTC (rev 221352)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/TextTrackCueList.cpp	2017-08-30 09:14:49 UTC (rev 221353)
@@ -106,6 +106,13 @@
     ASSERT_SORTED(m_vector.begin(), m_vector.end());
 }
 
+void TextTrackCueList::clear()
+{
+    m_vector.clear();
+    if (m_activeCues)
+        m_activeCues->m_vector.clear();
+}
+
 void TextTrackCueList::updateCueIndex(TextTrackCue& cue)
 {
     auto cuePosition = m_vector.begin() + cueIndex(cue);

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/TextTrackCueList.h (221352 => 221353)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/TextTrackCueList.h	2017-08-30 09:00:12 UTC (rev 221352)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/html/track/TextTrackCueList.h	2017-08-30 09:14:49 UTC (rev 221353)
@@ -46,6 +46,8 @@
     void remove(TextTrackCue&);
     void updateCueIndex(TextTrackCue&);
 
+    void clear();
+
     TextTrackCueList& activeCues();
 
 private:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to