- Revision
- 228886
- Author
- [email protected]
- Date
- 2018-02-21 11:36:47 -0800 (Wed, 21 Feb 2018)
Log Message
VTTCue constructor should use 'double' type for startTime / endTime
https://bugs.webkit.org/show_bug.cgi?id=182988
Reviewed by Eric Carlson.
Source/WebCore:
VTTCue constructor should use 'double' type for startTime / endTime, not
'unrestricted double':
- https://w3c.github.io/webvtt/#the-vttcue-interface
Otherwise, we end up potentially returning NaN for TextTrackCue.startTime / endTime,
even though those correctly use type 'double':
- https://html.spec.whatwg.org/multipage/media.html#texttrackcue
The new behavior is consistent with Firefox and Chrome.
No new tests, updated existing test.
* bindings/js/JSDOMConvertNumbers.h:
(WebCore::JSConverter<IDLDouble>::convert):
Add assertion to make sure our implementation never tries to return NaN
for an IDL attribute of type 'double'. This would be invalid as per Web
IDL spec and would crash if the NaN being returned was impure as JSValue
could not store it as a double.
* html/track/VTTCue.idl:
Update constructor parameters to use 'double' type instead of 'unrestricted
double', as per:
- https://w3c.github.io/webvtt/#the-vttcue-interface
LayoutTests:
Update existing test to reflect behavior change.
* media/track/track-add-remove-cue-expected.txt:
* media/track/track-add-remove-cue.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (228885 => 228886)
--- trunk/LayoutTests/ChangeLog 2018-02-21 19:27:52 UTC (rev 228885)
+++ trunk/LayoutTests/ChangeLog 2018-02-21 19:36:47 UTC (rev 228886)
@@ -1,3 +1,15 @@
+2018-02-21 Chris Dumez <[email protected]>
+
+ VTTCue constructor should use 'double' type for startTime / endTime
+ https://bugs.webkit.org/show_bug.cgi?id=182988
+
+ Reviewed by Eric Carlson.
+
+ Update existing test to reflect behavior change.
+
+ * media/track/track-add-remove-cue-expected.txt:
+ * media/track/track-add-remove-cue.html:
+
2018-02-21 Ms2ger <[email protected]>
Test gardening.
Modified: trunk/LayoutTests/media/track/track-add-remove-cue-expected.txt (228885 => 228886)
--- trunk/LayoutTests/media/track/track-add-remove-cue-expected.txt 2018-02-21 19:27:52 UTC (rev 228885)
+++ trunk/LayoutTests/media/track/track-add-remove-cue-expected.txt 2018-02-21 19:36:47 UTC (rev 228886)
@@ -48,14 +48,10 @@
EXPECTED (newCue.align == 'middle') OK
*** Create an old-style cue with an id.
-RUN(oldStyleCue = new VTTCue('sausage-cue', 33, 3.4, 'Sausage?'))
-EXPECTED (oldStyleCue.id == '') OK
-EXPECTED (oldStyleCue.startTime.toString() == 'NaN') OK
-EXPECTED (oldStyleCue.endTime == '33') OK
-*** Make sure the old-style cue is not inserted because its start time is not a number.
+RUN(invalidCue = new VTTCue('sausage-cue', 33, 3.4, 'Sausage?'))
+TypeError: The provided value is non-finite
+EXPECTED (window.invalidCue == 'undefined') OK
EXPECTED (testTrack.track.cues.length == '5') OK
-RUN(testTrack.track.addCue(oldStyleCue))
-EXPECTED (testTrack.track.cues.length == '5') OK
*** Remove a cue created with addCue().
RUN(testTrack.track.removeCue(textCue))
Modified: trunk/LayoutTests/media/track/track-add-remove-cue.html (228885 => 228886)
--- trunk/LayoutTests/media/track/track-add-remove-cue.html 2018-02-21 19:27:52 UTC (rev 228885)
+++ trunk/LayoutTests/media/track/track-add-remove-cue.html 2018-02-21 19:36:47 UTC (rev 228886)
@@ -62,14 +62,10 @@
testExpected("newCue.align", "middle");
consoleWrite("<br>*** Create an old-style cue with an id.");
- run("oldStyleCue = new VTTCue('sausage-cue', 33, 3.4, 'Sausage?')");
- testExpected("oldStyleCue.id", "");
- testExpected("oldStyleCue.startTime.toString()", "NaN");
- testExpected("oldStyleCue.endTime", 33);
- consoleWrite("*** Make sure the old-style cue is not inserted because its start time is not a number.");
+ run("invalidCue = new VTTCue('sausage-cue', 33, 3.4, 'Sausage?')");
+ testExpected("window.invalidCue", undefined);
+
testExpected("testTrack.track.cues.length", 5);
- run("testTrack.track.addCue(oldStyleCue)");
- testExpected("testTrack.track.cues.length", 5);
consoleWrite("<br>*** Remove a cue created with addCue().");
run("testTrack.track.removeCue(textCue)");
Modified: trunk/LayoutTests/media/track/track-cue-negative-timestamp-expected.txt (228885 => 228886)
--- trunk/LayoutTests/media/track/track-cue-negative-timestamp-expected.txt 2018-02-21 19:27:52 UTC (rev 228885)
+++ trunk/LayoutTests/media/track/track-cue-negative-timestamp-expected.txt 2018-02-21 19:36:47 UTC (rev 228886)
@@ -3,13 +3,13 @@
Test that cues with negative startTime are not added:
EXPECTED (testTrack.track.cues.length == '4') OK
-RUN(textCue = new VTTCue('sausage-cue', -3439332606, 3.4, 'Sausage?'))
+RUN(textCue = new VTTCue(-3439332606, 3.4, 'Sausage?'))
RUN(testTrack.track.addCue(textCue))
EXPECTED (testTrack.track.cues.length == '4') OK
Test that cues with negative startTime and negative endTime are not added:
EXPECTED (testTrack.track.cues.length == '4') OK
-RUN(textCue = new VTTCue('pepperoni-cue', -110, -3.4, 'Pepperoni?'))
+RUN(textCue = new VTTCue(-110, -3.4, 'Pepperoni?'))
RUN(testTrack.track.addCue(textCue))
EXPECTED (testTrack.track.cues.length == '4') OK
Modified: trunk/LayoutTests/media/track/track-cue-negative-timestamp.html (228885 => 228886)
--- trunk/LayoutTests/media/track/track-cue-negative-timestamp.html 2018-02-21 19:27:52 UTC (rev 228885)
+++ trunk/LayoutTests/media/track/track-cue-negative-timestamp.html 2018-02-21 19:36:47 UTC (rev 228886)
@@ -10,13 +10,13 @@
{
consoleWrite("Test that cues with negative startTime are not added:");
testExpected("testTrack.track.cues.length", 4);
- run("textCue = new VTTCue('sausage-cue', -3439332606, 3.4, 'Sausage?')");
+ run("textCue = new VTTCue(-3439332606, 3.4, 'Sausage?')");
run("testTrack.track.addCue(textCue)");
testExpected("testTrack.track.cues.length", 4);
consoleWrite("<br>Test that cues with negative startTime and negative endTime are not added:");
testExpected("testTrack.track.cues.length", 4);
- run("textCue = new VTTCue('pepperoni-cue', -110, -3.4, 'Pepperoni?')");
+ run("textCue = new VTTCue(-110, -3.4, 'Pepperoni?')");
run("testTrack.track.addCue(textCue)");
testExpected("testTrack.track.cues.length", 4);
Modified: trunk/Source/WebCore/ChangeLog (228885 => 228886)
--- trunk/Source/WebCore/ChangeLog 2018-02-21 19:27:52 UTC (rev 228885)
+++ trunk/Source/WebCore/ChangeLog 2018-02-21 19:36:47 UTC (rev 228886)
@@ -1,3 +1,34 @@
+2018-02-21 Chris Dumez <[email protected]>
+
+ VTTCue constructor should use 'double' type for startTime / endTime
+ https://bugs.webkit.org/show_bug.cgi?id=182988
+
+ Reviewed by Eric Carlson.
+
+ VTTCue constructor should use 'double' type for startTime / endTime, not
+ 'unrestricted double':
+ - https://w3c.github.io/webvtt/#the-vttcue-interface
+
+ Otherwise, we end up potentially returning NaN for TextTrackCue.startTime / endTime,
+ even though those correctly use type 'double':
+ - https://html.spec.whatwg.org/multipage/media.html#texttrackcue
+
+ The new behavior is consistent with Firefox and Chrome.
+
+ No new tests, updated existing test.
+
+ * bindings/js/JSDOMConvertNumbers.h:
+ (WebCore::JSConverter<IDLDouble>::convert):
+ Add assertion to make sure our implementation never tries to return NaN
+ for an IDL attribute of type 'double'. This would be invalid as per Web
+ IDL spec and would crash if the NaN being returned was impure as JSValue
+ could not store it as a double.
+
+ * html/track/VTTCue.idl:
+ Update constructor parameters to use 'double' type instead of 'unrestricted
+ double', as per:
+ - https://w3c.github.io/webvtt/#the-vttcue-interface
+
2018-02-21 Zalan Bujtas <[email protected]>
[RenderTreeBuilder] Move RenderTextFragment::willBeRemoved() mutation logic to RenderTreeBuilder
Modified: trunk/Source/WebCore/bindings/js/JSDOMConvertNumbers.h (228885 => 228886)
--- trunk/Source/WebCore/bindings/js/JSDOMConvertNumbers.h 2018-02-21 19:27:52 UTC (rev 228885)
+++ trunk/Source/WebCore/bindings/js/JSDOMConvertNumbers.h 2018-02-21 19:36:47 UTC (rev 228886)
@@ -360,6 +360,7 @@
static JSC::JSValue convert(Type value)
{
+ ASSERT(!std::isnan(value));
return JSC::jsNumber(value);
}
};
Modified: trunk/Source/WebCore/html/track/VTTCue.idl (228885 => 228886)
--- trunk/Source/WebCore/html/track/VTTCue.idl 2018-02-21 19:27:52 UTC (rev 228885)
+++ trunk/Source/WebCore/html/track/VTTCue.idl 2018-02-21 19:36:47 UTC (rev 228886)
@@ -25,7 +25,7 @@
[
Conditional=VIDEO_TRACK,
- Constructor(unrestricted double startTime, unrestricted double endTime, DOMString text),
+ Constructor(double startTime, double endTime, DOMString text),
ConstructorCallWith=ScriptExecutionContext,
JSGenerateToJSObject,
JSGenerateToNativeObject,
