third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html - Issue 1854553003: Convert webvtt regions tests from video-test.js to testharness.js based

Side by Side Diff: third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html

Issue 1854553003: Convert webvtt regions tests from video-test.js to testharness.js based (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: address review comments Created 4 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-constructor.html ('K') | « third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-constructor-expected.txt ('k') | third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display-expected.txt » ('j') | third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-dom-layout.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <html>	2 <title>Tests default rendering for TextTrackCues that belong to a VTTRegion.</ti tle>

3 <head>	3 <script src="../../media-controls.js"></script>

4 <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />	4 <script src="../../media-file.js"></script>

	5 <script src="../../../resources/testharness.js"></script>

	6 <script src="../../../resources/testharnessreport.js"></script>

	7 <video controls></video>

	8 <script>

	9 var testTrack;

	10 var region;

	11 var container;

	12 var totalVisibleLines;

5	13

6 <script src=../../media-controls.js></script>	14 var seekTimes = [0.2, 0.5, 1.0, 2.3, 3.0];

7 <script src=../../media-file.js></script>	15 var crtSeekTime = 0;
	philipj_slow 2016/04/06 12:40:04 Based on how it's used I guess this means "current Based on how it's used I guess this means "current seek time", but can you make it seekTimeIndex or something more explicit? Srirama 2016/04/07 10:40:07 Done. Show quoted text On 2016/04/06 12:40:04, philipj wrote: > Based on how it's used I guess this means "current seek time", but can you make > it seekTimeIndex or something more explicit? Done.
8 <!-- TODO(philipj): Convert test to testharness.js. crbug.com/588956

9 (Please avoid writing new tests using video-test.js) -->

10 <script src=../../video-test.js></script>

11 <script>

12 var testTrack;

13 var region;

14 var container;

15 var totalVisibleLines;

16	16

17 var seekTimes = [0.2, 0.5, 1.0, 2.3, 3.0];	17 function countVisibleLines(cueElement)

18 var crtSeekTime = 0;	18 {

	19 var cueRect = cueElement.getBoundingClientRect();

	20 var regionRect = region.getBoundingClientRect();

19	21

20 function countVisibleLines(cueElement)	22 var linesMatch = cueElement.textContent.match(/\n/g);

21 {	23 var linesCount = 1 + (linesMatch == null ? 0 : linesMatch.length);

22 var cueRect = cueElement.getBoundingClientRect();	24 var lineHeight = cueRect.height / linesCount;

23 var regionRect = region.getBoundingClientRect();

24	25

25 var linesMatch = cueElement.textContent.match(/\n/g);	26 var visibleLines = 0;

26 var linesCount = 1 + (linesMatch == null ? 0 : linesMatch.length);	27 for (i = 0; i < linesCount; ++i) {

27 var lineHeight = cueRect.height / linesCount;	28 var lineTop = cueRect.top + i * lineHeight;

	29 var lineBottom = cueRect.top + (i+1) * lineHeight;

28	30

29 var visibleLines = 0;	31 if (lineTop >= regionRect.top && lineBottom <= regionRect.bottom)

30 for (i = 0; i < linesCount; ++i) {	32 visibleLines++;

31 var lineTop = cueRect.top + i * lineHeight;	33 }

32 var lineBottom = cueRect.top + (i+1) * lineHeight;

33	34

34 if (lineTop >= regionRect.top && lineBottom <= regionRect.bottom)	35 return visibleLines;

35 visibleLines++;	36 }

36 }

37	37

38 return visibleLines;	38 async_test(function() {

39 }	39 var video = document.querySelector('video');

	40 video.src = findMediaFile('video', '../../content/test');

	41 var testTrack = document.createElement('track');

	42 testTrack.onload = this.step_func(function() {
	Srirama 2016/04/01 12:14:23 Not able to use var 't' here so i removed it every Not able to use var 't' here so i removed it everywhere and i am using 'this'. It says t.step_func not defined, may be 't' is overridden by some other variable somehow. philipj_slow 2016/04/06 12:19:40 It looks like you're just missing a "t" above, try Show quoted text On 2016/04/01 12:14:23, Srirama wrote: > Not able to use var 't' here so i removed it everywhere and i am using 'this'. > It says t.step_func not defined, may be 't' is overridden by some other variable > somehow. It looks like you're just missing a "t" above, try "async_test(function(t) {" Srirama 2016/04/07 10:40:07 Because of this issue i have removed 't' from ever Show quoted text On 2016/04/06 12:19:40, philipj wrote: > On 2016/04/01 12:14:23, Srirama wrote: > > Not able to use var 't' here so i removed it everywhere and i am using 'this'. > > It says t.step_func not defined, may be 't' is overridden by some other > variable > > somehow. > > It looks like you're just missing a "t" above, try "async_test(function(t) {" Because of this issue i have removed 't' from every place. Now i have added 't' whereever i can but as you can see, i am using "this.step_func" below as i cannot use 't' there even though i declare 't' here. Same is the case with one more test (vtt-region-dom-layout.html). philipj_slow 2016/04/07 11:07:39 That sounds weird, why couldn't you use t below? F Show quoted text On 2016/04/07 10:40:07, Srirama wrote: > On 2016/04/06 12:19:40, philipj wrote: > > On 2016/04/01 12:14:23, Srirama wrote: > > > Not able to use var 't' here so i removed it everywhere and i am using > 'this'. > > > It says t.step_func not defined, may be 't' is overridden by some other > > variable > > > somehow. > > > > It looks like you're just missing a "t" above, try "async_test(function(t) {" > > Because of this issue i have removed 't' from every place. Now i have added 't' > whereever i can but as you can see, i am using "this.step_func" below as i > cannot use 't' there even though i declare 't' here. Same is the case with one > more test (vtt-region-dom-layout.html). That sounds weird, why couldn't you use t below? For consistency, maybe just use this everywhere in this test? Srirama 2016/04/07 11:15:52 Don't know the exact reason, all other test cases Show quoted text On 2016/04/07 11:07:39, philipj wrote: > On 2016/04/07 10:40:07, Srirama wrote: > > On 2016/04/06 12:19:40, philipj wrote: > > > On 2016/04/01 12:14:23, Srirama wrote: > > > > Not able to use var 't' here so i removed it everywhere and i am using > > 'this'. > > > > It says t.step_func not defined, may be 't' is overridden by some other > > > variable > > > > somehow. > > > > > > It looks like you're just missing a "t" above, try "async_test(function(t) > {" > > > > Because of this issue i have removed 't' from every place. Now i have added > 't' > > whereever i can but as you can see, i am using "this.step_func" below as i > > cannot use 't' there even though i declare 't' here. Same is the case with one > > more test (vtt-region-dom-layout.html). > > That sounds weird, why couldn't you use t below? For consistency, maybe just use > this everywhere in this test? Don't know the exact reason, all other test cases are using just one level of step_func where as these two tests are using multiple levels of step_func, i thought that may be the only reason.
	43 video.addEventListener('canplaythrough', this.step_func(function() {

	44 video.removeEventListener('canplaythrough', this);
	philipj_slow 2016/04/06 12:40:04 I don't think this is the callback here, is it? ar I don't think this is the callback here, is it? arguments.callee might work, but if removing the listener isn't even needed for the test, you can just drop it. Or use oncanplaythrough and set it to null. Srirama 2016/04/07 10:40:07 Done. Show quoted text On 2016/04/06 12:40:04, philipj wrote: > I don't think this is the callback here, is it? arguments.callee might work, but > if removing the listener isn't even needed for the test, you can just drop it. > Or use oncanplaythrough and set it to null. Done.
	45 track = video.textTracks[0];

40	46

41 function testRegionsDisplay()	47 assert_equals(track.regions.length, 1);

42 {

43 video.removeEventListener('canplaythrough', testRegionsDisplay);

44	48

45 testTrack = video.textTracks[0];	49 region = textTrackDisplayElement(video, 'region');

	50 container = textTrackDisplayElement(video, 'region-container');

46	51

47 consoleWrite(" The text track has only one region ");	52 video.onseeked = this.step_func(function() {

48 testExpected("testTrack.regions.length", 1);	53 totalVisibleLines = 0;

49	54

50 try {	55 for (var i = 0; i < container.children.length; ++i) {

51 region = textTrackDisplayElement(video, 'region');	56 var cue = container.children[i];

52 container = textTrackDisplayElement(video, 'region-container');	57 totalVisibleLines += countVisibleLines(cue);;

53 } catch(e) {	58 }

54 consoleWrite(e);

55 }

56	59

57 consoleWrite("<br> Inspecting cues displayed within region");	60 assert_less_than_equal(totalVisibleLines, track.regions[0].heigh t);

58	61

59 waitForEvent("seeked", inspectRegionTree);	62 if (crtSeekTime == seekTimes.length)

60 seekVideo();	63 this.done();

61 }	64 else

62	65 video.currentTime = seekTimes[crtSeekTime++];

63 function seekVideo()	66 });

64 {	67 video.currentTime = seekTimes[crtSeekTime++];

65 consoleWrite("");	68 }));

66 run("video.currentTime = " + seekTimes[crtSeekTime++]);	69 });

67 }	70 testTrack.src = '../captions-webvtt/captions-regions.vtt';

68	71 testTrack.kind = 'captions';

69 function inspectRegionTree()	72 testTrack.default = true;

70 {	73 video.appendChild(testTrack);

71 consoleWrite("Total cues in region: " + container.children.length);	74 });

72 totalVisibleLines = 0;	75 </script>

73

74 for (var i = 0; i < container.children.length; ++i) {

75 var cue = container.children[i];

76 var cueVisibleLines = countVisibleLines(cue);

77 consoleWrite("Cue content is: " + cue.textContent);

78 consoleWrite("Cue lines visible from this cue: " + cueVisibleLin es);

79

80 totalVisibleLines += cueVisibleLines;

81 }

82

83 testExpected("totalVisibleLines <= testTrack.regions[0].height", tru e);

84

85 if (crtSeekTime == seekTimes.length)

86 endTest();

87 else

88 seekVideo();

89 }

90

91 function startTest()

92 {

93 if (!window.VTTRegion) {

94 failTest();

95 return;

96 }

97

98 findMediaElement();

99

100 video.src = findMediaFile('video', '../../content/test');

101 video.addEventListener('canplaythrough', testRegionsDisplay);

102 }

103

104 </script>

105 </head>

106 <body>

107 <p>Tests default rendering for TextTrackCues that belong to a VTTRegion. </p>

108 <video controls>

109 <track src="../captions-webvtt/captions-regions.vtt" kind="captions" default onload="startTest()">

110 </video>

111 </body>

112 </html>

OLD	NEW