LayoutTests/fast/text/fontWidthVariant.html - Issue 263363005: Initial patch for FontVariant, variant like HalfWidth should take Complex Path

Side by Side Diff: LayoutTests/fast/text/fontWidthVariant.html

Issue 263363005: Initial patch for FontVariant, variant like HalfWidth should take Complex Path (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@emoji_android

Patch Set: Adding test case based on testharness.js Created 6 years, 7 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

OLD	NEW
(Empty)
	1 <!DOCTYPE HTML>
	Dominik Röttsches 2014/05/13 08:45:34 Please change the file name to be consistent with Please change the file name to be consistent with surrounding file names. Lower case with hyphens. The patch is missing expected result for the new test case.
	2 <meta charset="utf-8">

	3 <html>

	4 <head>

	5 <style type="text/css" id="styleElement">

	6 </style>

	7 <style>

	8 @font-face {

	9 font-family: CSSHWOrientationTest;

	10 src: url(../../resources/CSSHWOrientationTest.ttf) format("truetype");
	Dominik Röttsches 2014/05/13 08:45:34 Please take a look at LayoutTests/third_party/Libe Please take a look at LayoutTests/third_party/Libertine for an example on how to add a new third party font. Make a new separate patch about adding the font, please add dpranke as reviewer. You need to add a license file and a readme file with justification why the font is needed.
	11 }

	12

	13 html {

	14 -webkit-writing-mode: vertical-rl;

	15 text-rendering: geometricPrecision;

	16 }

	17

	18 p {

	19 font-family: "CSSHWOrientationTest", sans;

	20 font-size: 24px;

	21 }

	22

	23 .combine {

	24 -webkit-text-combine: horizontal;

	25 }

	26

	27 </style>

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

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

	30 <script type="text/javascript">

	31 setup({ "explicit_done": true });

	32 function updateQuery() {
	Dominik Röttsches 2014/05/13 08:45:34 Why "updateQuery()?" - Please think about better n Why "updateQuery()?" - Please think about better naming.
	33 var styleElement = document.getElementById("styleElement");

	34 var spanElement = document.getElementById("variantCombine");

	35

	36 var theCSSprop_1 = spanElement.offsetHeight;
	Dominik Röttsches 2014/05/13 08:45:34 Please clean up code before uploading to make it e Please clean up code before uploading to make it easier for reviewers to understand what you're trying to do. 1) Remove commented code. 2) Remove debugging code. 3) Fix indentation. The logic is indeed one way to test whether there is a difference between the height before and after applying the class. But we don't know into how many blocks. Also, you're only testing one combination of digits. Please follow my instructions on how to design this test more closely.
	37 //alert(theCSSprop_1);

	38

	39 // Apply class having webkit-text-combine property

	40 document.getElementById('variantCombine').className = 'combine';

	41

	42 var theCSSprop_2 = spanElement.offsetHeight;

	43 //alert(theCSSprop_2);

	44

	45 test(function() {

	46 assert_true(theCSSprop_2 < theCSSprop_1,

	47 "New query doesn't match, property not applied.")
	Dominik Röttsches 2014/05/13 08:45:34 Needs a better assert failure description. The pro Needs a better assert failure description. The property is applied, but its rendering is incorrect - is probably what you mean here.
	48 },

	49 "Testing that webkit-text-combine is appplied");

	50

	51 // reset class

	52 spanElement.className = "";

	53 }

	54 </script>

	55 </head>

	56

	57 <body onload="updateQuery(); done();">

	58 <span>This test verifies that webkit-text-combin is applied.</span>
	Dominik Röttsches 2014/05/13 08:45:34 Please spell the CSS property correctly, including Please spell the CSS property correctly, including the hyphen. What we're testing is not exactly that the property is applied, but that the applied property renders visually correctly. Testing whether the property is applied would mean that we're testing CSS style resolution etc. But we know that this part works. What we're really testing here is whether the rendering is correct. For this, I don't think you need a dynamic test that compares the property before and after. You can apply the -webkit-text-combine CSS from the start, and then just measure the height and compare it to the font size. Here: If it's closer to 24 * 2 = 48, we know it's broken into two boxes.
	59 <div id="log"></div>

	60 <p>

	61 文字<span id="variantCombine">90</span>年
	Dominik Röttsches 2014/05/13 08:45:34 Do we need the filler text before and after the "9 Do we need the filler text before and after the "90"?
	62 </p>

	63 </body>

	64 </html>

OLD	NEW

« no previous file with comments | « no previous file | LayoutTests/resources/CSSHWOrientationTest.ttf » ('j') | no next file with comments »