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

CoordinateParser should handle NumState.invalid first #640

Merged
merged 9 commits into from Jan 14, 2020

Conversation

wieslawsoltes
Copy link
Contributor

@wieslawsoltes wieslawsoltes commented Jan 13, 2020

Reference Issue

e-polyline-004.svg

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">
    <title>Stop processing on invalid data</title>
    <desc>`text` and everything after it should be ignored</desc>

    <!-- should be covered -->
    <path id="path1" d="M 20 40 L 160 180 L 30 150" fill="none" stroke="red"/>

    <polyline id="polyline1" points="20 40 160 180 30 150 text" fill="none" stroke="green"/>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

e-polygon-004.svg

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg">
    <title>Stop processing on invalid data</title>
    <desc>`text` and everything after it should be ignored</desc>

    <!-- should be covered -->
    <path id="path1" d="M 20 40 L 160 180 L 30 150" fill="red" stroke="red"/>

    <polygon id="polygon1" points="20 40 160 180 30 150 text" fill="green" stroke="green"/>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

What does this implement/fix? Explain your changes.

Any other comments?

Copy link
Member

@mrbean-bremen mrbean-bremen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Can you add a release note entry for the fix? Also, a test would be nice (e.g. test image + PNG).
@H1Gdev - can you have a second look, please?

@wieslawsoltes
Copy link
Contributor Author

Can you add a release note entry for the fix? Also, a test would be nice (e.g. test image + PNG).

Added.

Copy link
Contributor

@H1Gdev H1Gdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wieslawsoltes
@mrbean-bremen

add review comment.

Tests/W3CTestSuite/PassingTests.txt Show resolved Hide resolved
@wieslawsoltes
Copy link
Contributor Author

Fixed review comments

@mrbean-bremen
Copy link
Member

Thanks!

@mrbean-bremen mrbean-bremen merged commit 2a38deb into svg-net:master Jan 14, 2020
mrbean-bremen pushed a commit that referenced this pull request Jan 21, 2020
* Fixed regression introduce by #640 * Added tests
wieslawsoltes added a commit to wieslawsoltes/SVG that referenced this pull request Jan 23, 2020
* Fixed regression introduce by svg-net#640 * Added tests
@wieslawsoltes wieslawsoltes deleted the CoordinateParserFix branch January 24, 2020 17:38
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.

None yet

3 participants