RESOLVED FIXED 203770
Fix the defaults of the SVGFEConvolveMatrixElement properties
https://bugs.webkit.org/show_bug.cgi?id=203770
Summary Fix the defaults of the SVGFEConvolveMatrixElement properties
Said Abou-Hallawa
Reported 2019-11-01 16:01:54 PDT
Created attachment 382642 [details] test case Open the attached test case. Expected results: Default value for feConvolveMatrix divisor property is: 1 Default value for feConvolveMatrix orderX property is: 3 Default value for feConvolveMatrix orderY property is: 3 The specs links are: https://www.w3.org/TR/SVG11/filters.html#feConvolveMatrixElementDivisorAttribute https://www.w3.org/TR/SVG11/filters.html#feConvolveMatrixElementOrderAttribute
Attachments
test case (704 bytes, image/svg+xml)
2019-11-01 16:01 PDT, Said Abou-Hallawa
no flags
Patch (19.15 KB, patch)
2019-11-01 16:14 PDT, Said Abou-Hallawa
sabouhallawa: review?
Safari 15.5 differs from other browsers (623.60 KB, image/png)
2022-05-31 13:53 PDT, Ahmad Saleem
no flags
Said Abou-Hallawa
Comment 1 2019-11-01 16:14:57 PDT
Simon Fraser (smfr)
Comment 2 2019-11-13 16:30:36 PST
Comment on attachment 382647 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382647&action=review > Source/WebCore/svg/SVGFEConvolveMatrixElement.cpp:74 > + if (parseNumberOptionalNumber(value, x, y) && (x < 1 || y < 1)) > document().accessSVGExtensions().reportWarning("feConvolveMatrix: problem parsing order=\"" + value + "\". Filtered element will not be displayed."); > + m_orderX->setBaseValInternal(x); > + m_orderY->setBaseValInternal(y); So we parse the values but set them anyway? Seems odd. > Source/WebCore/svg/SVGFEConvolveMatrixElement.cpp:121 > + if (parseNumberOptionalNumber(value, x, y) && (x <= 0 || y <= 0)) > document().accessSVGExtensions().reportWarning("feConvolveMatrix: problem parsing kernelUnitLength=\"" + value + "\". Filtered element will not be displayed."); > + > + m_kernelUnitLengthX->setBaseValInternal(x); > + m_kernelUnitLengthY->setBaseValInternal(y); Ditto.
Ahmad Saleem
Comment 3 2022-05-31 13:53:19 PDT
Created attachment 459904 [details] Safari 15.5 differs from other browsers This issue is still present and reproducible in Safari 15.5 on macOS 12.4 while Chrome Canary 104 matches with Firefox Nightly 103. Thanks!
Radar WebKit Bug Importer
Comment 4 2024-02-08 15:13:54 PST
Said Abou-Hallawa
Comment 5 2025-08-04 18:02:01 PDT
Said Abou-Hallawa
Comment 6 2025-08-05 12:04:53 PDT
(In reply to Simon Fraser (smfr) from comment #2) > Comment on attachment 382647 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=382647&action=review > > > Source/WebCore/svg/SVGFEConvolveMatrixElement.cpp:74 > > + if (parseNumberOptionalNumber(value, x, y) && (x < 1 || y < 1)) > > document().accessSVGExtensions().reportWarning("feConvolveMatrix: problem parsing order=\"" + value + "\". Filtered element will not be displayed."); > > + m_orderX->setBaseValInternal(x); > > + m_orderY->setBaseValInternal(y); > > So we parse the values but set them anyway? Seems odd. > I do not think this odd. If the web developer decides to set the attribute to out of range value we should respect that. All we need is to handle this as an error when building the filter effect in SVGFEConvolveMatrixElement::build(). When setting the attribute to an invalid number (like setting it to a string) or removing the attribute we should reset its value to the default value. > > Source/WebCore/svg/SVGFEConvolveMatrixElement.cpp:121 > > + if (parseNumberOptionalNumber(value, x, y) && (x <= 0 || y <= 0)) > > document().accessSVGExtensions().reportWarning("feConvolveMatrix: problem parsing kernelUnitLength=\"" + value + "\". Filtered element will not be displayed."); > > + > > + m_kernelUnitLengthX->setBaseValInternal(x); > > + m_kernelUnitLengthY->setBaseValInternal(y); > > Ditto.
EWS
Comment 7 2025-08-05 12:37:09 PDT
Committed 298250@main (5a05da2f8229): <https://commits.webkit.org/298250@main> Reviewed commits have been landed. Closing PR #48935 and removing active labels.
Ahmad Saleem
Comment 8 2025-08-09 09:28:48 PDT
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/54221
Note You need to log in before you can comment on or make changes to this bug.