third_party/WebKit/LayoutTests/animations/transition-zoomed-length.html - Issue 2562643002: Avoid visually transitioning on length CSS properties when zoom changes

Side by Side Diff: third_party/WebKit/LayoutTests/animations/transition-zoomed-length.html

Issue 2562643002: Avoid visually transitioning on length CSS properties when zoom changes (Closed)

Patch Set: lint Created 4 years ago

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

Jump to:

« no previous file with comments | « no previous file | third_party/WebKit/Source/core/animation/css/CSSAnimatableValueFactory.cpp » ('j') | third_party/WebKit/Source/core/animation/css/CSSAnimatableValueFactory.cpp » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

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

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

	4 <style>

	5 #target {

	6 transition: 1s;

	7 border-style: solid;

	8 outline-style: solid;

	9 column-rule-style: solid;

	10 }

	11 </style>

	12 <div id="target"></div>

	13 <script>

	14 var lengthProperties = [
	suzyh_UTC10 (ex-contributor) 2016/12/12 04:30:13 Is there a canonical source we can get this list o Is there a canonical source we can get this list of properties from, instead of hard-coding it here? alancutter (OOO until 2018) 2016/12/13 02:36:34 Not one I would be comfortable relying on not chan Show quoted text On 2016/12/12 at 04:30:13, suzyh wrote: > Is there a canonical source we can get this list of properties from, instead of hard-coding it here? Not one I would be comfortable relying on not changing unexpectedly. Generally it's fine for tests to have hard coded values for the purpose of regression testing.
	15 'baselineShift',

	16 'borderBottomWidth',

	17 'borderLeftWidth',

	18 'borderRightWidth',

	19 'borderTopWidth',

	20 'bottom',

	21 'cx',

	22 'cy',

	23 'flexBasis',

	24 'height',

	25 'left',

	26 'letterSpacing',

	27 'marginBottom',

	28 'marginLeft',

	29 'marginRight',

	30 'marginTop',

	31 'maxHeight',

	32 'maxWidth',

	33 'minHeight',

	34 'minWidth',

	35 'offsetDistance',

	36 'outlineOffset',

	37 'outlineWidth',

	38 'paddingBottom',

	39 'paddingLeft',

	40 'paddingRight',

	41 'paddingTop',

	42 'perspective',

	43 'r',

	44 'right',

	45 'rx',

	46 'ry',

	47 'shapeMargin',

	48 'strokeDashoffset',

	49 'strokeWidth',

	50 'top',

	51 'verticalAlign',

	52 'webkitBorderHorizontalSpacing',

	53 'webkitBorderVerticalSpacing',

	54 'columnGap',

	55 'columnRuleWidth',

	56 'columnWidth',

	57 'webkitPerspectiveOriginX',

	58 'webkitPerspectiveOriginY',

	59 'webkitTransformOriginX',

	60 'webkitTransformOriginY',

	61 'webkitTransformOriginZ',

	62 'width',

	63 'wordSpacing',

	64 'x',

	65 'y',

	66 'lineHeight',

	67 ];

	68 var expected = {};

	69 test(() => {
	suzyh_UTC10 (ex-contributor) 2016/12/12 04:30:13 This is not a test. Why can this not be written wi This is not a test. Why can this not be written without the "test(() => { ... }, 'Test setup');" surrounds? alancutter (OOO until 2018) 2016/12/13 02:36:34 Without the test() wrapper if this code throws an Show quoted text On 2016/12/12 at 04:30:13, suzyh wrote: > This is not a test. Why can this not be written without the "test(() => { ... }, 'Test setup');" surrounds? Without the test() wrapper if this code throws an error it won't be caught by the test harness and the test won't fail. Added comment.
	70 for (var property of lengthProperties) {

	71 target.style[property] = '10px';

	72 expected[property] = getComputedStyle(target)[property];

	73 }

	74 if (window.internals) {
	suzyh_UTC10 (ex-contributor) 2016/12/12 04:30:13 Please add a comment to explain what these two opt Please add a comment to explain what these two options are for. Is there a reason not to use only "target.style.zoom = 2"? alancutter (OOO until 2018) 2016/12/13 02:36:34 The working group is trying to get rid of the zoom Show quoted text On 2016/12/12 at 04:30:13, suzyh wrote: > Please add a comment to explain what these two options are for. Is there a reason not to use only "target.style.zoom = 2"? The working group is trying to get rid of the zoom property so I don't want to rely on it. I should probably just not use it at all and error if internals doesn't exist.
	75 internals.setZoomFactor(2);

	76 } else {

	77 target.style.zoom = 2;

	78 }

	79 }, 'Test setup');

	80

	81 for (var property of lengthProperties) {

	82 test(() => {

	83 assert_equals(getComputedStyle(target)[property], expected[property]);

	84 }, 'Computed value of transitionable ' + property + ' should not change when z oom changes');

	85 }

	86 </script>

OLD	NEW