Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parsing number values using scientific notation w/ a capital E. #5731

Merged
merged 2 commits into from
May 31, 2019
Merged

Fix parsing number values using scientific notation w/ a capital E. #5731

merged 2 commits into from
May 31, 2019

Conversation

rodovich
Copy link
Contributor

Fixes parsing numbers like 1.2E3 → 1200 in fabric.reNum.

Per https://www.w3.org/TR/css-syntax-3/#number-token-diagram, either lowercase e or uppercase E can be used in scientific notation numbers. (That page is called "CSS Syntax" but the SVG spec at https://www.w3.org/TR/SVG/types.html#InterfaceSVGNumber delegates to that section of the CSS spec.)

I updated the existing tests for parsing matrix and points. points already worked fine because it uses parseFloat and not fabric.reNum, but the matrix test failed before fixing the regular expression.

@asturur
Copy link
Member

asturur commented May 31, 2019

i m ok with this PR.
I wonder if svg 1.1 or 1.2 should be considered stopped at the CSS specs that were available those years.

@asturur asturur merged commit 6888a38 into fabricjs:master May 31, 2019
@asturur
Copy link
Member

asturur commented May 31, 2019

thanks!

@asturur asturur mentioned this pull request Jun 1, 2019
@rodovich rodovich deleted the fix-number-regex branch June 3, 2019 13:46
thiagocunha pushed a commit to thiagocunha/fabric.js that referenced this pull request Nov 18, 2019
…abricjs#5731)

* Test parsing number values using scientific notation w/ a capital E.

* Fix parsing number values using scientific notation w/ a capital E.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants