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

Add basic marker on shape tests. #10993

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Tavmjong
Copy link
Contributor

@Tavmjong Tavmjong commented May 14, 2018

Basic marker on shape tests.

title="NAME_OF_REVIEWER"
href="mailto:EMAIL OR http://CONTACT_PAGE" />
<!-- YYYY-MM-DD -->
<html:link rel="help"
Copy link
Contributor

@svgeesus svgeesus May 19, 2018

Choose a reason for hiding this comment

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

remove rel=help from test references (applies to all references in this PR)

href="https://www.w3.org/TR/SVG2/painting.html#Markers"/>
<html:link rel="match" href="marker-shape-001-ref.svg" />
<metadata class="flags">TOKENS</metadata>
<desc class="assert">TEST ASSERTION</desc>
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible, add an actual test assertion. If not, delete the boilerplate assertion.

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

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

Technical content of the tests looks correct (rects and equivalent paths)

xmlns:xlink="http://www.w3.org/1999/xlink"
xmlns:html="http://www.w3.org/1999/xhtml">
<g id="testmeta">
<title>Hatch: Basic hatch.</title>
Copy link
Contributor

Choose a reason for hiding this comment

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

These tests are not about hatch

@wpt-pr-bot wpt-pr-bot removed the request for review from boggydigital August 9, 2018 06:54
Copy link
Contributor

@AmeliaBR AmeliaBR left a comment

Choose a reason for hiding this comment

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

Remove the unused font style information.

It shouldn't prevent merging of this PR, but:
There should really be tests for all the basic shapes, and those will need to be more specific than just comparing against a matching path (because some browsers currently paint too many markers for arcs in paths).

@AmeliaBR
Copy link
Contributor

AmeliaBR commented Dec 2, 2019

See discussion in w3c/svgwg#753 about possibly changing some of the spec behavior, which could affect these tests.

@Tavmjong
Copy link
Contributor Author

I've added circle/ellipse tests. And removed unused style element.

…d-marker at start/end of path equivalent for shapes.

Use straight lines for markers in reference SVGs to avoid rendering error by some browsers (too many markers on curves).
Fixed bad SVG for circle/ellipse test SVGs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants