Bug 203770 – Fix the defaults of the SVGFEConvolveMatrixElement properties

• Component: SVG
• Status: RESOLVED FIXED

[Said Abou-Hallawa]

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

[Said Abou-Hallawa]

Created attachment 382647 [details]
Patch

[Simon Fraser (smfr)]

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]

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]

<rdar://problem/122586298>

[Said Abou-Hallawa]

Pull request: https://github.com/WebKit/WebKit/pull/48935

[Said Abou-Hallawa]

(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]

Committed 298250@main (5a05da2f8229): <https://commits.webkit.org/298250@main>

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

[Ahmad Saleem]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/54221