third_party/WebKit/LayoutTests/media/controls/overflow-fully-hidden.html - Issue 2499013003: Media Controls: don't "cut" controls if fully hidden.

Side by Side Diff: third_party/WebKit/LayoutTests/media/controls/overflow-fully-hidden.html

Issue 2499013003: Media Controls: don't "cut" controls if fully hidden. (Closed)

Patch Set: add tests Created 4 years, 1 month ago

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

Jump to:

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html>

	3 <script>

	4 internals.settings.setViewportEnabled(true);

	5 internals.settings.setViewportMetaEnabled(true);

	6 </script>

	7 <head>

	8 <title>Test that overflow-x: hidden with a large viewport doesn't clip the contr ols</title>

	9 <meta name='viewport' content='width=800'>

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

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

	12 <script src="../media-file.js"></script>

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

	14 </head>

	15 <style>

	16 body {

	17 overflow-x: hidden;

	18 }

	19 </style>

	20 <body>

	21 <video controls width=400 style='padding: 900px'></video>

	22 <video controls width=800 style='padding: 900px'></video>

	23 <video controls width=1200 style='padding: 900px'></video>

	24 <video controls width=600 style='padding: 900px'></video>

	25 </body>

	26 <script>

	27 test(_ => {

	28 const expectedWidth = [

	29 "400px",

	30 "800px",

	31 "1200px",

	32 "600px",

	33 ];

	34

	35 var videos = document.querySelectorAll('video');
	eae 2016/11/17 16:40:33 This doesn't really matter here but please keep in This doesn't really matter here but please keep in mind that in many cases, including this, querySelectorAll returns a live node list thus this loop is O(n^2). Either cache the length or change the loop to be a for in or a for loop like this to avoid that. for (var videoElement, i = 0; videoElement = videos[i]; i++) { videoElement.src = findMediaFile('video', 'content/test'); } mlamouri (slow - plz ping) 2016/11/18 14:53:50 Didn't realise this. Thanks :) Show quoted text On 2016/11/17 at 16:40:33, eae wrote: > This doesn't really matter here but please keep in mind that in many cases, including this, querySelectorAll returns a live node list thus this loop is O(n^2). > > Either cache the length or change the loop to be a for in or a for loop like this to avoid that. > > for (var videoElement, i = 0; videoElement = videos[i]; i++) { > videoElement.src = findMediaFile('video', 'content/test'); > } Didn't realise this. Thanks :)
	36 for (var i=0; i < videos.length; ++i) {

	37 videos[i].src = findMediaFile('video', 'content/test');

	38 }

	39

	40 var forceLayout = document.body.offsetHeight;

	41

	42 for (var i=0; i < videos.length; ++i) {

	43 var controls = mediaControlsButton(videos[i], 'panel');

	44 assert_equals(getComputedStyle(controls).width, expectedWidth[i]);
	eae 2016/11/17 16:40:33 Try to avoid data-driven tests like this. They ten Try to avoid data-driven tests like this. They tend to be hard to read and debug. Instead how about something like this: assert_equals(getComputedStyle(mediaControlsButton(videos[0], 'panel')).width, '400px'); assert_equals(getComputedStyle(mediaControlsButton(videos[1], 'panel')).width, '800px'); assert_equals(getComputedStyle(mediaControlsButton(videos[2], 'panel')).width, '1200px'); assert_equals(getComputedStyle(mediaControlsButton(videos[3], 'panel')).width, '600px'); mlamouri (slow - plz ping) 2016/11/18 14:53:50 I actually prefer data-driver tests because it all Show quoted text On 2016/11/17 at 16:40:33, eae wrote: > Try to avoid data-driven tests like this. They tend to be hard to read and debug. Instead how about something like this: > > assert_equals(getComputedStyle(mediaControlsButton(videos[0], 'panel')).width, '400px'); > assert_equals(getComputedStyle(mediaControlsButton(videos[1], 'panel')).width, '800px'); > assert_equals(getComputedStyle(mediaControlsButton(videos[2], 'panel')).width, '1200px'); > assert_equals(getComputedStyle(mediaControlsButton(videos[3], 'panel')).width, '600px'); I actually prefer data-driver tests because it allows for more tests. Though, given the size, I used your suggestion.
	45 }

	46 });

	47 </script>

	48 </html>

OLD	NEW

« no previous file with comments | « no previous file | third_party/WebKit/Source/core/layout/LayoutMedia.cpp » ('j') | no next file with comments »