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 SVGElement.ownerSVGElement. #1879

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add tests for SVGElement.ownerSVGElement. #1879

wants to merge 1 commit into from

Conversation

heycam
Copy link
Contributor

@heycam heycam commented Jun 6, 2015

Please let me know if this is the best way to handle scripted SVG tests. It seemed easier to do everything from an HTML document.

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/5202

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 21, 2015

Reviewed on critic

@nikosandronikos
Copy link
Member

critic looks to be gone? Are the comments archived anywhere else? If not, could you summarise here please?

@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 15, 2016



Issue in svg/types/SVGElement.ownerSVGElement-01.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12203
--------------------------------------------------------------------------------
29|function q(selector) { return document.querySelector(selector); }
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  Not a big fan of this kind of function; it hurts readability and doesn't save
  all that much.


Issue in svg/types/SVGElement.ownerSVGElement-01.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12204
--------------------------------------------------------------------------------
48|    if (typeof t[i] == "string") {
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  ===


Issue in svg/types/SVGElement.ownerSVGElement-01.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12205
--------------------------------------------------------------------------------
63|container.remove();
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  Is this widely supported now?


Issue in svg/types/SVGElement.ownerSVGElement-01.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12206
--------------------------------------------------------------------------------
30|var topSVG = q("#top-svg");
31|var innerSVG = q("#inner-svg");
32|var tests = [
33|  // [Element or selector used to find element,
34|  //  expected ownerSVGElement value,
35|  //  description]
36|  [topSVG, null, "outer <svg> element"],
37|  ["rect", topSVG, "non-<svg> child of outer <svg> element"],
38|  ["title", topSVG, "non-<svg> descendant of outer <svg> element"],
39|  [innerSVG, topSVG, "inner <svg> descendant of outer <svg> element"],
40|  ["polygon", innerSVG, "non-<svg> descendant of inner <svg> element"],
41|  ["#svg-in-fo", null, "outer <svg> in foreignObject"],
42|  ["#inner-svg-in-fo", "#svg-in-fo", "inner <svg> child of outer <svg> in foreignObject"],
43|];
44|
45|// Resolve the selectors to actual elements.
46|tests.forEach(function(t) {
47|  for (var i = 0; i < 2; i++) {
48|    if (typeof t[i] == "string") {
49|      t[i] = q(t[i]);
50|    }
51|  }
52|});
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  Should all go into setup()


Issue in svg/types/SVGElement.ownerSVGElement-01.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12207
--------------------------------------------------------------------------------
74|}, "non-svg element with no parent");
75|test(function() {
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  Empty line


Issue in svg/types/SVGElement.ownerSVGElement-01.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12208
--------------------------------------------------------------------------------
4|<link rel="help" href="http://www.w3.org/TR/SVG2/types.html#__svg__SVGElement__ownerSVGElement">
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  Please use the ED


Issue in svg/types/SVGElement.ownerSVGElement-02.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12209
--------------------------------------------------------------------------------
4|<link rel="help" href="http://www.w3.org/TR/SVG2/types.html#__svg__SVGElement__ownerSVGElement">
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  ED


Issue in svg/types/SVGElement.ownerSVGElement-02.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12210
--------------------------------------------------------------------------------
9|  <iframe src="helper-SVGElement.ownerSVGElement-02.svg" onload="run()"></iframe>
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  run() can be called before it's defined; put the script first.


Issue in svg/types/SVGElement.ownerSVGElement-02.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12211
--------------------------------------------------------------------------------
18|  }, "root svg element");
19|  test(function() {
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  Empty line


Issue in svg/types/SVGElement.ownerSVGElement-02.html:
  https://critic.hoppipolla.co.uk/showcomment?chain=12212
--------------------------------------------------------------------------------
11|<script>
12|function run() {
--------------------------------------------------------------------------------
Ms2ger at 2015-07-21 13:35 +02:00:
  This test will silently pass if the load event isn't fired (in time).

@sideshowbarker
Copy link
Member

@nikosandronikos is the OK to be merged?

@sideshowbarker
Copy link
Member

Oh actually I see now this seems to be waiting on @heycam to make changes in response to review comments

@foolip
Copy link
Member

foolip commented Apr 16, 2018

@heycam, do you plan to update this? (/me is triaging very old PRs)

@karlcow
Copy link
Contributor

karlcow commented Dec 10, 2024

see #49608

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.

8 participants