LayoutTests/fullscreen/rendering/style-defaults-iframe.html - Issue 974783002: Match the Fullscreen spec's CSS as far as currently practical

Side by Side Diff: LayoutTests/fullscreen/rendering/style-defaults-iframe.html

Issue 974783002: Match the Fullscreen spec's CSS as far as currently practical (Closed) Base URL: svn://svn.chromium.org/blink/trunk

Patch Set: tests Created 5 years, 9 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 | Annotate | Revision Log

« LayoutTests/fullscreen/full-screen-render-inline-expected.html ('K') | « LayoutTests/fullscreen/parent-flow-inline-with-block-child-expected.html ('k') | LayoutTests/fullscreen/rendering/style-defaults-root.html » ('j') | LayoutTests/fullscreen/rendering/style-defaults-root.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <title>User-agent level style sheet defaults for an iframe</title>
	Julien - ping for review 2015/03/04 01:07:37 Using <title> is useless in test. If it is meaning Using <title> is useless in test. If it is meaningful, it should be dumped in the final output. If not, it should be removed. Also the description doesn't match what you're trying to do. You're not testing the default at all, you're making sure they can't be overridden. philipj_slow 2015/03/04 07:15:28 With testharness.js the <title> becomes the test t Show quoted text On 2015/03/04 01:07:37, Julien Chaffraix - PST wrote: > Using <title> is useless in test. If it is meaningful, it should be dumped in > the final output. If not, it should be removed. With testharness.js the <title> becomes the test title, equivalent to putting it after the function in async_test. It is included in the test expectations ("PASS User-agent level style sheet defaults for an iframe") but those don't need to be checked in for all-passing testharness.js tests. Show quoted text > Also the description doesn't match what you're trying to do. You're not testing > the default at all, you're making sure they can't be overridden. I've renamed the tests to ua-style-.html, does that work? I think it makes sense to test the defaults and overriding them in the same test, because trying to override them is how we verify that !important is included. Julien - ping for review* 2015/03/04 17:50:22 Can you include the output of these tests? I think Show quoted text On 2015/03/04 07:15:28, philipj_UTC7 wrote: > On 2015/03/04 01:07:37, Julien Chaffraix - PST wrote: > > Using <title> is useless in test. If it is meaningful, it should be dumped in > > the final output. If not, it should be removed. > > With testharness.js the <title> becomes the test title, equivalent to putting it > after the function in async_test. It is included in the test expectations ("PASS > User-agent level style sheet defaults for an iframe") but those don't need to be > checked in for all-passing testharness.js tests. Can you include the output of these tests? I think I understand what you said but I would like to see the output for myself before making up my mind. (Also I would argue it's a bad decision from the W3C harness to have this magical behavior for <title> but that's another debate). Show quoted text > > Also the description doesn't match what you're trying to do. You're not > testing > > the default at all, you're making sure they can't be overridden. > > I've renamed the tests to ua-style-.html, does that work? I think it makes > sense to test the defaults and overriding them in the same test, because trying > to override them is how we verify that !important is included. I didn't have a gripe with the previous naming but the new name is better indeed. I meant that the content of <title> doesn't match what you're doing. You're not testing the defaults, you're testing that they cannot be overridden. Those are 2 different tests and the description matches the former when you're actually testing the latter. philipj_slow* 2015/03/05 04:05:06 There's a hook to prevent all-passing testharness. Show quoted text On 2015/03/04 17:50:22, Julien Chaffraix - PST wrote: > On 2015/03/04 07:15:28, philipj_UTC7 wrote: > > On 2015/03/04 01:07:37, Julien Chaffraix - PST wrote: > > > Using <title> is useless in test. If it is meaningful, it should be dumped > in > > > the final output. If not, it should be removed. > > > > With testharness.js the <title> becomes the test title, equivalent to putting > it > > after the function in async_test. It is included in the test expectations > ("PASS > > User-agent level style sheet defaults for an iframe") but those don't need to > be > > checked in for all-passing testharness.js tests. > > Can you include the output of these tests? I think I understand what you said > but I would like to see the output for myself before making up my mind. There's a hook to prevent all-passing testharness.js expectations from being checked in, but see below for what they look like after some more renaming. Show quoted text > (Also I would argue it's a bad decision from the W3C harness to have this > magical behavior for <title> but that's another debate). > > > > Also the description doesn't match what you're trying to do. You're not > > testing > > > the default at all, you're making sure they can't be overridden. > > > > I've renamed the tests to ua-style-.html, does that work? I think it makes > > sense to test the defaults and overriding them in the same test, because > trying > > to override them is how we verify that !important is included. > > I didn't have a gripe with the previous naming but the new name is better > indeed. > > I meant that the content of <title> doesn't match what you're doing. You're not > testing the defaults, you're testing that they cannot be overridden. Those are 2 > different tests and the description matches the former when you're actually > testing the latter. OK, just name of the section in the spec wasn't clear. It was hard to come up with a title for the override+border test, so I've split those. The three tests now have these test expectations: This is a testharness.js-based test. PASS Fullscreen UA style sheet removes iframe border Harness: the test ran to completion. This is a testharness.js-based test. PASS Fullscreen UA style sheet cannot be overriden by author style Harness: the test ran to completion. This is a testharness.js-based test. PASS Fullscreen UA style sheet does not apply to the :root element Harness: the test ran to completion. Julien - ping for review* 2015/03/05 18:19:03 OK, I think that's OK. It's kinda weird to dump th Show quoted text On 2015/03/05 04:05:06, philipj_UTC7 wrote: > On 2015/03/04 17:50:22, Julien Chaffraix - PST wrote: > > On 2015/03/04 07:15:28, philipj_UTC7 wrote: > > > On 2015/03/04 01:07:37, Julien Chaffraix - PST wrote: > > > > Using <title> is useless in test. If it is meaningful, it should be dumped > > in > > > > the final output. If not, it should be removed. > > > > > > With testharness.js the <title> becomes the test title, equivalent to > putting > > it > > > after the function in async_test. It is included in the test expectations > > ("PASS > > > User-agent level style sheet defaults for an iframe") but those don't need > to > > be > > > checked in for all-passing testharness.js tests. > > > > Can you include the output of these tests? I think I understand what you said > > but I would like to see the output for myself before making up my mind. > > There's a hook to prevent all-passing testharness.js expectations from being > checked in, but see below for what they look like after some more renaming. > > > (Also I would argue it's a bad decision from the W3C harness to have this > > magical behavior for <title> but that's another debate). > > > > > > Also the description doesn't match what you're trying to do. You're not > > > testing > > > > the default at all, you're making sure they can't be overridden. > > > > > > I've renamed the tests to ua-style-.html, does that work? I think it makes > > > sense to test the defaults and overriding them in the same test, because > > trying > > > to override them is how we verify that !important is included. > > > > I didn't have a gripe with the previous naming but the new name is better > > indeed. > > > > I meant that the content of <title> doesn't match what you're doing. You're > not > > testing the defaults, you're testing that they cannot be overridden. Those are > 2 > > different tests and the description matches the former when you're actually > > testing the latter. > > OK, just name of the section in the spec wasn't clear. It was hard to come up > with a title for the override+border test, so I've split those. The three tests > now have these test expectations: > > This is a testharness.js-based test. > PASS Fullscreen UA style sheet removes iframe border > Harness: the test ran to completion. > > This is a testharness.js-based test. > PASS Fullscreen UA style sheet cannot be overriden by author style > Harness: the test ran to completion. > > This is a testharness.js-based test. > PASS Fullscreen UA style sheet does not apply to the :root element > Harness: the test ran to completion. OK, I think that's OK. It's kinda weird to dump the description after PASS. philipj_slow* 2015/03/06 02:43:40 This is because it's possible to have multiple tes Show quoted text On 2015/03/05 18:19:03, Julien Chaffraix - PST wrote: > On 2015/03/05 04:05:06, philipj_UTC7 wrote: > > On 2015/03/04 17:50:22, Julien Chaffraix - PST wrote: > > > On 2015/03/04 07:15:28, philipj_UTC7 wrote: > > > > On 2015/03/04 01:07:37, Julien Chaffraix - PST wrote: > > > > > Using <title> is useless in test. If it is meaningful, it should be > dumped > > > in > > > > > the final output. If not, it should be removed. > > > > > > > > With testharness.js the <title> becomes the test title, equivalent to > > putting > > > it > > > > after the function in async_test. It is included in the test expectations > > > ("PASS > > > > User-agent level style sheet defaults for an iframe") but those don't need > > to > > > be > > > > checked in for all-passing testharness.js tests. > > > > > > Can you include the output of these tests? I think I understand what you > said > > > but I would like to see the output for myself before making up my mind. > > > > There's a hook to prevent all-passing testharness.js expectations from being > > checked in, but see below for what they look like after some more renaming. > > > > > (Also I would argue it's a bad decision from the W3C harness to have this > > > magical behavior for <title> but that's another debate). > > > > > > > > Also the description doesn't match what you're trying to do. You're not > > > > testing > > > > > the default at all, you're making sure they can't be overridden. > > > > > > > > I've renamed the tests to ua-style-*.html, does that work? I think it > makes > > > > sense to test the defaults and overriding them in the same test, because > > > trying > > > > to override them is how we verify that !important is included. > > > > > > I didn't have a gripe with the previous naming but the new name is better > > > indeed. > > > > > > I meant that the content of <title> doesn't match what you're doing. You're > > not > > > testing the defaults, you're testing that they cannot be overridden. Those > are > > 2 > > > different tests and the description matches the former when you're actually > > > testing the latter. > > > > OK, just name of the section in the spec wasn't clear. It was hard to come up > > with a title for the override+border test, so I've split those. The three > tests > > now have these test expectations: > > > > This is a testharness.js-based test. > > PASS Fullscreen UA style sheet removes iframe border > > Harness: the test ran to completion. > > > > This is a testharness.js-based test. > > PASS Fullscreen UA style sheet cannot be overriden by author style > > Harness: the test ran to completion. > > > > This is a testharness.js-based test. > > PASS Fullscreen UA style sheet does not apply to the :root element > > Harness: the test ran to completion. > > OK, I think that's OK. It's kinda weird to dump the description after PASS. This is because it's possible to have multiple tests in a single file, and in that case you get one PASS (hopefully) with the test title on each line.
	3 <script src="../../resources/testharness.js"></script>

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

	5 <script src="../trusted-event.js"></script>

	6 <style>

	7 iframe {

	8 /* Try to override everything which is !important in the UA style sheet. */

	9 position: relative !important;

	10 top: auto !important;

	11 right: auto !important;

	12 bottom: auto !important;

	13 left: auto !important;

	14 margin: 10px !important;

	15 box-sizing: content-box !important;

	16 min-width: 50% !important;

	17 max-width: 50% !important;

	18 min-height: 50% !important;

	19 max-height: 50% !important;

	20 object-fit: none !important;

	21 border: 1px solid black; /* not !important in the UA style sheet */

	22 }

	23 </style>

	24 <div id="log"></div>

	25 <iframe></iframe>

	26 <script>

	27 async_test(function()

	28 {

	29 var iframe = document.querySelector("iframe");

	30

	31 document.addEventListener("fullscreenchange", this.step_func_done(function()

	32 {

	33 var style = getComputedStyle(iframe);

	34 // All properties except border should match the UA style sheet.

	35 assert_equals(style.position, "fixed");

	36 assert_equals(style.top, "0px");

	37 assert_equals(style.right, "0px");

	38 assert_equals(style.bottom, "0px");

	39 assert_equals(style.left, "0px");

	40 assert_equals(style.margin, "0px");

	41 assert_equals(style.boxSizing, "border-box");

	42 assert_equals(style.minWidth, "0px");

	43 assert_equals(style.maxWidth, "none");

	44 assert_equals(style.minHeight, "0px");

	45 assert_equals(style.maxHeight, "none");

	46 assert_equals(style.width, window.innerWidth + "px");

	47 assert_equals(style.height, window.innerHeight + "px");
	Julien - ping for review 2015/03/04 01:07:37 width and height are not overriden width and height are not overriden philipj_slow 2015/03/04 07:15:28 I removed them when I noticed it doesn't make any Show quoted text On 2015/03/04 01:07:37, Julien Chaffraix - PST wrote: > width and height are not overriden I removed them when I noticed it doesn't make any difference, which I think is because I'm testing the computed style and not the specified style I'd really like to test. I've added them back.
	48 assert_equals(style.objectFit, "contain");

	49 assert_equals(style.flex, "0 0 auto");
	Julien - ping for review 2015/03/04 01:07:37 Nor is flex Nor is flex philipj_slow 2015/03/04 07:15:28 Done. Show quoted text On 2015/03/04 01:07:37, Julien Chaffraix - PST wrote: > Nor is flex Done.
	50 assert_equals(style.transform, "none");
	Julien - ping for review 2015/03/04 01:07:37 Nor transform. Nor transform. philipj_slow 2015/03/04 07:15:28 Done. Show quoted text On 2015/03/04 01:07:37, Julien Chaffraix - PST wrote: > Nor transform. Done.
	51 assert_equals(style.borderStyle, "solid");

	52 }));

	53

	54 trusted_request(iframe);

	55 });

	56 </script>

OLD	NEW