[webkit-reviews] review granted: [Bug 220971] REGRESSION (r243266): SVGStopElement does not react upon 'offset' attribute changes : [Attachment 418398] Patch

[bugzilla-daemon at webkit.org]

Said Abou-Hallawa <sabouhallawa at apple.com> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 220971: REGRESSION (r243266): SVGStopElement does not react upon 'offset'
attribute changes
https://bugs.webkit.org/show_bug.cgi?id=220971

Attachment 418398: Patch

https://bugs.webkit.org/attachment.cgi?id=418398&action=review




--- Comment #3 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 418398
  --> https://bugs.webkit.org/attachment.cgi?id=418398
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418398&action=review

> LayoutTests/svg/custom/deep-dynamic-updates-expected.svg:3
> +<svg width="450" height="450" xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink">

xlink namespace was removed in SVG2. The plan is keep it in old tests but not
to use it in new tests.

> LayoutTests/svg/custom/deep-dynamic-updates-expected.svg:19
> +<use x="25" y="25" width="400" height="400" xlink:href="#symbol"/>

XLink URL reference attribute was deprecated in SVG2.

>> LayoutTests/svg/custom/deep-dynamic-updates.svg:3
>> +<svg width="450" height="450" xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink" onload="setTimeout(runTest, 0)">
> 
> This delay setTimeout(runTest, 0) is crucial. Otherwise we change the stop
offset before the first rendering, and thus don't test the "dynamic updates" of
offsetAttr code path at all...

Alternatively, requestAnimationFrame() could be used to call runTest(). I think
it is even more reliable than setTimeout().

    requestAnimationFrame(() => {
	document.getElementById("stop1").offset.baseVal = 0.3;
	if (window.testRunnner)
	    window.testRunner.notifyDone();
    });

> LayoutTests/svg/custom/deep-dynamic-updates.svg:19
>  <script>

Please make the <script> tag be the last element in the SVG document.