Bug 206019 - Setting currentScale to non-finite values should throw TypeError
Summary: Setting currentScale to non-finite values should throw TypeError
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-09 11:21 PST by Sunny He
Modified: 2020-01-21 10:58 PST (History)
15 users (show)

See Also:


Attachments
Patch (5.06 KB, patch)
2020-01-09 11:29 PST, Sunny He
no flags Details | Formatted Diff | Diff
Patch (5.21 KB, patch)
2020-01-09 13:13 PST, Sunny He
no flags Details | Formatted Diff | Diff
Patch (4.61 KB, patch)
2020-01-17 11:10 PST, Sunny He
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sunny He 2020-01-09 11:21:57 PST
SVG2 spec 5.14.2 Interface SVGSVGElement (https://www.w3.org/TR/SVG2/struct.html#__svg__SVGSVGElement__currentScale) defines currentScale to be float, not unrestricted float. Chrome and Firefox throw TypeError if currentScale is set to NaN or Inf, WebKit silently sets currentScale to NaN.
Comment 1 Radar WebKit Bug Importer 2020-01-09 11:22:23 PST
<rdar://problem/58450454>
Comment 2 Sunny He 2020-01-09 11:29:10 PST
Created attachment 387247 [details]
Patch
Comment 3 Said Abou-Hallawa 2020-01-09 12:40:54 PST
Comment on attachment 387247 [details]
Patch

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

> Source/WebCore/ChangeLog:6
> +        Align SVGElement currentScale definition to SVG2 section 5.14.2.

Please add a link to the specs: https://www.w3.org/TR/SVG2/struct.html#InterfaceSVGSVGElement.

> LayoutTests/svg/dom/set-currentScale-nonfinite.html:15
> +    svgvar.currentScale = 1.0;
> +    shouldBe('svgvar.currentScale', '1.0')

Maybe you need to add semicolon at the end each statement just for consistency.

> LayoutTests/svg/dom/set-currentScale-nonfinite.html:46
> +        testFailed('set currentScale to Infinity without error')

svgvar.currentScale is set to character 'a' not Infinity.

> LayoutTests/svg/dom/set-currentScale-nonfinite.html:49
> +            testPassed('set currentScale to Infinity throws TypeError')

Ditto.
Comment 4 Sunny He 2020-01-09 13:13:35 PST
Created attachment 387258 [details]
Patch
Comment 5 Dean Jackson 2020-01-11 12:50:51 PST
Comment on attachment 387258 [details]
Patch

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

> LayoutTests/svg/dom/set-currentScale-nonfinite.html:24
> +    try {
> +        svgvar.currentScale = undefined;
> +        testFailed('set currentScale to undefined without error');
> +    } catch(e) {
> +        if (e instanceof TypeError)
> +            testPassed('set currentScale to undefined throws TypeError');
> +    }
> +    shouldBe('svgvar.currentScale', '1.0');

This is fine, but maybe you could have avoided the duplication:

const testIncorrectScaleValue = (value, description) => {
  try {
     svgvar.currentScale = value;
     testFailed(`Setting currentScale to ${description} should have thrown an exception.`);
  } catch (e) {
     if (e instanceof TypeError)
         testPassed(`Setting currentScale to ${description} threw an exception.`);
  }
  shouldBe('svgvar.currentScale', '1.0');
};

testIncorrectScaleValue(undefined, "undefined");
testIncorrectScaleValue(NaN, "NaN");
testIncorrectScaleValue(Infinity, "Infinity");
testIncorrectScaleValue("a", "a string");
Comment 6 Sunny He 2020-01-17 11:10:36 PST
Created attachment 388065 [details]
Patch
Comment 7 WebKit Commit Bot 2020-01-21 10:58:42 PST
Comment on attachment 388065 [details]
Patch

Clearing flags on attachment: 388065

Committed r254863: <https://trac.webkit.org/changeset/254863>
Comment 8 WebKit Commit Bot 2020-01-21 10:58:44 PST
All reviewed patches have been landed.  Closing bug.