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 tests for rel, rellist, download, and text on SVGAElement #10275

Merged
merged 5 commits into from
Aug 7, 2018

Conversation

dstorey
Copy link
Contributor

@dstorey dstorey commented Apr 2, 2018

These are based on the equivalent tests in HTML's Anchor folder with minimal changes to put in SVG template and creating elements in the SVG namespace.

These tests are a precursor to moving these members to a mixin defined in HTML, and as such are expected to fail in current implementations.

based on html/semantics/text-level-semantics/the-a-element/a-download-click.html
const link = frame.contentDocument.querySelector("#blob-url");
link.href.baseVal = URL.createObjectURL(blob);

link.click();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: we haven't moved click() into the HTMLOrSVGElement mixin yet, so this will fail even if download is supported. I wasn't sure how to force click the download link without this and didn't want to change too much from the original test. We've discussed adding click() to the mixin, which I think we probably should do (will be trivial to implement in Edge at least)

@ghost
Copy link

ghost commented Apr 2, 2018

Build PASSED

Started: 2018-04-02 23:31:18
Finished: 2018-04-02 23:45:16

View more information about this build on:

@longsonr
Copy link
Contributor

longsonr commented Apr 5, 2018

the first argument to test_rellist('A', link_support_table['a']); in rellist-feature-detection.svg should be 'a' and not 'A'

With that change Firefox with the patch in https://bugzilla.mozilla.org/show_bug.cgi?id=1451823 passes these tests apart from the download test which is noted as broken above.

See https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1c1cfc763df659af3fbad8b832a4273e6cd0da0&selectedJob=172137761 for more details.

Note there is another wpt that needs changing: testing/web-platform/tests/dom/lists/DOMTokenList-coverage-for-attributes.html I have a fix in the patch in https://bugzilla.mozilla.org/show_bug.cgi?id=1451823 for that.

@dstorey
Copy link
Contributor Author

dstorey commented Apr 6, 2018

@longsonr thanks for the spot, and the implementation. I've just updated it.

For the click() issue, are you interested in implementing that in Firefox? If so I'll open a PR to add it to Edge and we'll have the two implementations. I'll also update the HTML GitHub issue for HTMLorSVGElement to get click() added to that mixin.

Otherwise we'll need to find another way to test download attribute.

@longsonr
Copy link
Contributor

longsonr commented Apr 7, 2018

While I could implement click() support provided it was added to the specification and there was agreement to implement, which it sounds as if there might be. Wouldn't it be better for now to just change the link.click() call to this:

link.dispatchEvent(new Event('click'));

That works in my patched Firefox build

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c90e8e53643dfbb5711b1c257c7b3682454df90

@longsonr
Copy link
Contributor

My fix landed in Firefox. The WPT test change is in #10473

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.

With changes suggested by @longsonr included, this lgtm

@svgeesus svgeesus merged commit 60f5bae into web-platform-tests:master Aug 7, 2018
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.

4 participants