third_party/WebKit/LayoutTests/svg/repaint/color-fill-currentColor-and-css.html - Issue 1455153003: Fix animation of 'color' w/ currentColor for SVG 'fill' and 'stroke'

Side by Side Diff

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

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: third_party/WebKit/LayoutTests/svg/repaint/color-fill-currentColor-and-css.html

Issue 1455153003: Fix animation of 'color' w/ currentColor for SVG 'fill' and 'stroke' (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 5 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:

View unified diff | Download patch

« no previous file with comments | « no previous file | third_party/WebKit/LayoutTests/svg/repaint/color-fill-currentColor-and-css-expected.png » ('j') | third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.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="../../fast/repaint/resources/text-based-repaint.js"></script>

	3 <script>

	4 window.testIsAsync = true;

	5

	6 var rootSVGElement;
	fs 2015/12/15 11:36:24 You don't need this. You don't need this.
	7

	8 function svgLoaded() {
	fs 2015/12/15 11:36:24 ...and hence not this. ...and hence not this.
	9 rootSVGElement = document.getElementById("svg");

	10 }

	11

	12

	13 function runTimer() {

	14 finishRepaintTest();

	15 }

	16

	17 function repaintTest() {

	18 cirlce = rootSVGElement.ownerDocument.getElementsByTagName("circle")[0];
	fs 2015/12/15 11:36:24 'rootSVGElement.ownerDocument' could just be writt 'rootSVGElement.ownerDocument' could just be written as 'document'. I also don't see any reason to put cirlce and bound on the global object (use var to make them local).
	19 bound = circle.getBoundingClientRect();

	20

	21 if (window.eventSender) {

	22 eventSender.mouseMoveTo(bound.left + 40, bound.top + 40);
	fs 2015/12/15 11:36:24 Do we really need 'hover' to trigger the bug, or c Do we really need 'hover' to trigger the bug, or could we just do: circle.style.color = 'cyan'; or circle.classList.add('someclass'); or similar instead to trigger the transition (or animation)?
	23 }

	24

	25 setTimeout(runTimer, 2000);
	fs 2015/12/15 11:36:24 s/runTimer/finishRepaintTest/ - but I suspect this s/runTimer/finishRepaintTest/ - but I suspect this will be flaky, so you should really seek a method that does not require these kind of mechanism. Maybe you could use Element.animate(...) instead and use currentTime (and maybe pause())?
	26 }

	27 </script>

	28 <style>

	29 circle {

	30 fill: currentColor;

	31 transition: 2s;

	32 color: blue;

	33 }

	34

	35 circle:hover {

	36 color: cyan;

	37 }

	38 </style>

	39 <body onload="runRepaintAndPixelTest()">
	fs 2015/12/15 11:36:24 You can do: onload = runRepaintAndPixelTest; wit You can do: onload = runRepaintAndPixelTest; within the <script> instead. Then you can remove the <body>.
	40 <svg id="svg" xmlns="http://www.w3.org/2000/svg" width=90 height=90 onload="svgL oaded()">
	fs 2015/12/15 11:36:24 Per above you no longer need 'id' or 'onload' - 'x Per above you no longer need 'id' or 'onload' - 'xmlns' is not needed and the default dimension ought to be fine. so this could just be: <svg>
	41 <circle id="circle" r="40" cx="40" cy="40" />
	fs 2015/12/15 11:36:24 The 'id' isn't used so that can be dropped. I also The 'id' isn't used so that can be dropped. I also don't think this needs to be a circle, so I'd suggest a simple 100x100 <rect> instead.
	42 </svg>

	43 </body>

OLD	NEW