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

[svg 2 text] add scripted tests for length and indexed access #10172

Merged
merged 1 commit into from Mar 31, 2018

Conversation

Projects
None yet
5 participants
@liamquin
Copy link

liamquin commented Mar 26, 2018

The text and tspan elements seem to be the only ones in SVG itself affected by this change. The spec only demands read-only access so that's all that's tested here.

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 26, 2018

Build PASSED

Started: 2018-03-26 01:25:44
Finished: 2018-03-26 01:35:21

View more information about this build on:

@AmeliaBR

This comment has been minimized.

Copy link
Contributor

AmeliaBR commented Mar 26, 2018

Thanks for getting this started, Liam! Comments:

The text and tspan elements seem to be the only ones in SVG itself affected by this change.

All the SVGXXXList interfaces should now support indexed access and setting; not sure whether there are any existing tests. Other instances where the lists are exposed on element classes:

  • the animated transform lists for transform, patternTransform, and gradientTransform
  • the points and animatedPoints lists in polygons and polylines
  • requiredExtensions and systemLanguage (exposed on most elements) represented as SVGStringList

The spec only demands read-only access

animVal is read-only. baseVal is read-write.

The reference URL should be to the interface definition, not the layout notes:
https://svgwg.org/svg2-draft/single-page.html#text-InterfaceSVGTextPositioningElement
and
https://svgwg.org/svg2-draft/single-page.html#types-TermListInterface

(or the multipage versions of the URLs) (or the TR versions) (Do we have a policy on what URL should be used as a reference?)

The meta descriptions should be unique for each test file. (Also, it's length property, not length() method.)

@liamquin

This comment has been minimized.

Copy link
Author

liamquin commented Mar 26, 2018

@svgeesus

This comment has been minimized.

Copy link
Contributor

svgeesus commented Mar 31, 2018

Amelia is correct, animVal is read-only because it is the result of animation. but baseVal.value is read-write and further, changing it will update animVal

@svgeesus
Copy link
Contributor

svgeesus left a comment

These look good.

It would be useful to check that the lists can be edited in script (and the length changes), and also a visual test that the dx dy and rotate actally do something.

But my comment is more "these tests remind me we should also test that". The tests here are good.

@svgeesus svgeesus merged commit 3589b85 into web-platform-tests:master Mar 31, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@AmeliaBR

This comment has been minimized.

Copy link
Contributor

AmeliaBR commented Mar 31, 2018

Did you make the changes you had been going to, Liam?

@liamquin

This comment has been minimized.

Copy link
Author

liamquin commented Apr 1, 2018

@liamquin

This comment has been minimized.

Copy link
Author

liamquin commented Apr 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.