[SVG] Default for x1, y1 and y2 is 0% for LinearGradient #6731

Ahmad-S792 · 2022-11-22T13:22:24Z

`7a2abd4`

[SVG] Default for x1, y1 and y2 is 0% for LinearGradient

[SVG] Default for x1, y1 and y2 is 0% for LinearGradient
https://bugs.webkit.org/show_bug.cgi?id=248212

Reviewed by Nikolas Zimmermann.

This patch is to align Webkit with Blink / Chrome and Gecko / Firefox.

Merge - https://chromium.googlesource.com/chromium/blink/+/219dff463987f53c22f5f96f35a794246a13538d

As per the spec, if the x1|y1|y2 attribute is not specified,
the effect is as if a value of "0%" were specified.

* Source/WebCore/svg/SVGLinearGradientElement.h: Update default values for x1, y1 and y2
* LayoutTests/svg/gradient/linear-gradient-default-length.html: Add Test Case
* LayoutTests/svg/gradient/linear-gradient-default-length-expected.txt: Add Test Case Expectations
* LayoutTests/svg/gradient/linear-gradient-y2-default-length.svg: Added Test Case
* LayoutTests/svg/gradient/linear-gradient-y2-default-length-expected.svg: Added Test Case Expectations
* LayoutTests/svg/gradient/linear-gradient-y1-default-length.svg: Added Test Case
* LayoutTests/svg/gradient/linear-gradient-y1-default-length-expected.svg: Added Test Case Expectations
* LayoutTests/svg/gradient/linear-gradient-x1-default-length.svg: Added Test Case
* LayoutTests/svg/gradient/linear-gradient-x1-default-length-expected.svg: Added Test Case Expectations

Canonical link: https://commits.webkit.org/257032@main

a757bd5

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk1
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
✅ 🛠 🧪 merge	✅ 🛠 watch	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-22T13:22:40Z

EWS run on previous version of this PR (hash 59a6b64)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 🧪 win~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	⏳ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	✅ 🧪 api-ios	~~🧪 api-mac~~	⏳ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	~~🧪 mac-wk2~~
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-22T13:23:46Z

EWS run on previous version of this PR (hash 59a6b64)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

nikolaszimmermann

Any WPT test that already covers this?
A reftest that checks the rendering would be useful, to proof the visual effect is correct too.

I checked via some keywords to see if there is any WPT but I couldn't manage to find it. :-( You want new tests beside this test for linear gradient? Or want me to change "expected file" of this test?

webkit-early-warning-system · 2022-11-22T13:58:31Z

EWS run on previous version of this PR (hash 70463bd)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 🧪 win~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	⏳ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	~~🧪 api-ios~~	~~🧪 api-mac~~	⏳ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	~~🧪 mac-wk2~~
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-22T13:59:32Z

EWS run on previous version of this PR (hash 70463bd)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

Ahmad-S792 · 2022-11-22T13:59:53Z

@nikolaszimmermann - I added the tests but it seems that it might not be an easy fix and would need some more changes. Since prior to my latest push, the added test was still failing after the changes. So I think it might need more fixing.

nikolaszimmermann

Forgot to say, svg/gradients is a more natural place for these tests. However, I still wonder if there is WPT coverage?

Source/WebCore/svg/SVGLinearGradientElement.h

webkit-early-warning-system · 2022-11-22T14:29:34Z

EWS run on previous version of this PR (hash 07252d7)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

Ahmad-S792 · 2022-11-22T14:30:06Z

Forgot to say, svg/gradients is a more natural place for these tests. However, I still wonder if there is WPT coverage?

Do you want me to change them to gradient or wait till whole EWS to run to see if there is any WPT coverage and then do one final push to get everything sorted?

webkit-early-warning-system · 2022-11-22T14:31:13Z

EWS run on previous version of this PR (hash 07252d7)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 🧪 win~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	⏳ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	~~🧪 api-ios~~	~~🧪 api-mac~~	⏳ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	~~🧪 mac-wk2~~
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	~~🧪 mac-wk2-stress~~

webkit-early-warning-system · 2022-11-22T14:34:51Z

EWS run on previous version of this PR (hash 2bd9d54)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	❌ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	❌ 🧪 gtk-wk2
	✅ 🧪 api-ios	~~🧪 api-mac~~	✅ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	❌ 🧪 mac-wk2
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-22T14:34:59Z

EWS run on current version of this PR (hash 2bd9d54)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

Ahmad-S792 · 2022-11-22T14:35:13Z

@nikolaszimmermann - Moved to SVG/Gradient and also fixed the other stuff. :-)

webkit-early-warning-system · 2022-11-22T18:59:17Z

EWS run on current version of this PR (hash a757bd5)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk1
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
✅ 🛠 🧪 merge	✅ 🛠 watch	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

Ahmad-S792 · 2022-11-24T11:36:48Z

@nikolaszimmermann - Moved to SVG/Gradient and also fixed the other stuff. :-)

Now it is good to go for your review. mac-wk1 is platform / infra issue.

nikolaszimmermann

LGTM, r=me if EWS is fine.

[SVG] Default for x1, y1 and y2 is 0% for LinearGradient https://bugs.webkit.org/show_bug.cgi?id=248212 Reviewed by Nikolas Zimmermann. This patch is to align Webkit with Blink / Chrome and Gecko / Firefox. Merge - https://chromium.googlesource.com/chromium/blink/+/219dff463987f53c22f5f96f35a794246a13538d As per the spec, if the x1|y1|y2 attribute is not specified, the effect is as if a value of "0%" were specified. * Source/WebCore/svg/SVGLinearGradientElement.h: Update default values for x1, y1 and y2 * LayoutTests/svg/gradient/linear-gradient-default-length.html: Add Test Case * LayoutTests/svg/gradient/linear-gradient-default-length-expected.txt: Add Test Case Expectations * LayoutTests/svg/gradient/linear-gradient-y2-default-length.svg: Added Test Case * LayoutTests/svg/gradient/linear-gradient-y2-default-length-expected.svg: Added Test Case Expectations * LayoutTests/svg/gradient/linear-gradient-y1-default-length.svg: Added Test Case * LayoutTests/svg/gradient/linear-gradient-y1-default-length-expected.svg: Added Test Case Expectations * LayoutTests/svg/gradient/linear-gradient-x1-default-length.svg: Added Test Case * LayoutTests/svg/gradient/linear-gradient-x1-default-length-expected.svg: Added Test Case Expectations Canonical link: https://commits.webkit.org/257032@main

webkit-commit-queue · 2022-11-26T12:01:37Z

Committed 257032@main (7a2abd4): https://commits.webkit.org/257032@main

Reviewed commits have been landed. Closing PR #6731 and removing active labels.

Ahmad-S792 self-assigned this Nov 22, 2022

Ahmad-S792 added the SVG For bugs in the SVG implementation. label Nov 22, 2022

Ahmad-S792 requested a review from nikolaszimmermann November 22, 2022 13:28

Ahmad-S792 marked this pull request as ready for review November 22, 2022 13:28

nikolaszimmermann reviewed Nov 22, 2022

View reviewed changes

Ahmad-S792 force-pushed the fix248212 branch from 59a6b64 to 70463bd Compare November 22, 2022 13:55

Ahmad-S792 force-pushed the fix248212 branch from 70463bd to 07252d7 Compare November 22, 2022 14:28

nikolaszimmermann reviewed Nov 22, 2022

View reviewed changes

Source/WebCore/svg/SVGLinearGradientElement.h Outdated Show resolved Hide resolved

Source/WebCore/svg/SVGLinearGradientElement.h Outdated Show resolved Hide resolved

Source/WebCore/svg/SVGLinearGradientElement.h Outdated Show resolved Hide resolved

Ahmad-S792 force-pushed the fix248212 branch from 07252d7 to 2bd9d54 Compare November 22, 2022 14:34

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 22, 2022

Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Nov 22, 2022

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 22, 2022

Ahmad-S792 force-pushed the fix248212 branch from 2bd9d54 to a757bd5 Compare November 22, 2022 18:56

Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Nov 22, 2022

Ahmad-S792 requested a review from shallawa November 26, 2022 10:09

nikolaszimmermann approved these changes Nov 26, 2022

View reviewed changes

Ahmad-S792 added the merge-queue Applied to send a pull request to merge-queue label Nov 26, 2022

webkit-early-warning-system force-pushed the fix248212 branch from a757bd5 to 7a2abd4 Compare November 26, 2022 12:01

webkit-early-warning-system merged commit 7a2abd4 into WebKit:main Nov 26, 2022

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 26, 2022

Ahmad-S792 deleted the fix248212 branch December 14, 2022 21:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SVG] Default for x1, y1 and y2 is 0% for LinearGradient #6731

[SVG] Default for x1, y1 and y2 is 0% for LinearGradient #6731

Ahmad-S792 commented Nov 22, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

nikolaszimmermann left a comment •

edited by Ahmad-S792

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

Ahmad-S792 commented Nov 22, 2022

nikolaszimmermann left a comment

webkit-early-warning-system commented Nov 22, 2022 •

edited

Ahmad-S792 commented Nov 22, 2022

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

Ahmad-S792 commented Nov 22, 2022

webkit-early-warning-system commented Nov 22, 2022 •

edited

Ahmad-S792 commented Nov 24, 2022

nikolaszimmermann left a comment

webkit-commit-queue commented Nov 26, 2022

[SVG] Default for x1, y1 and y2 is 0% for LinearGradient #6731

[SVG] Default for x1, y1 and y2 is 0% for LinearGradient #6731

Conversation

Ahmad-S792 commented Nov 22, 2022 • edited by webkit-early-warning-system

7a2abd4

webkit-early-warning-system commented Nov 22, 2022 • edited

webkit-early-warning-system commented Nov 22, 2022 • edited

nikolaszimmermann left a comment • edited by Ahmad-S792

Choose a reason for hiding this comment

webkit-early-warning-system commented Nov 22, 2022 • edited

webkit-early-warning-system commented Nov 22, 2022 • edited

Ahmad-S792 commented Nov 22, 2022

nikolaszimmermann left a comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Nov 22, 2022 • edited

Ahmad-S792 commented Nov 22, 2022

webkit-early-warning-system commented Nov 22, 2022 • edited

webkit-early-warning-system commented Nov 22, 2022 • edited

webkit-early-warning-system commented Nov 22, 2022 • edited

Ahmad-S792 commented Nov 22, 2022

webkit-early-warning-system commented Nov 22, 2022 • edited

Ahmad-S792 commented Nov 24, 2022

nikolaszimmermann left a comment

Choose a reason for hiding this comment

webkit-commit-queue commented Nov 26, 2022

Ahmad-S792 commented Nov 22, 2022 •

edited by webkit-early-warning-system

`7a2abd4`

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

nikolaszimmermann left a comment •

edited by Ahmad-S792

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited