1908991 - Minor cleanup to tests for dynamic removal of href from SVG image element

Status: RESOLVED FIXED

(Core :: SVG, task)

Description

[Daniel Holbert [:dholbert]]

Filing this for some tweaks that I'd like to make to the tests that landed in bug 1908625.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: Bug 1908991 part 1: Various tweaks to image-remove-href-* WPT tests. r?longsonr

In particular, this patch...

• Changes the geometry to use sizes of 2 & 4 instead of 1.5 & 3 (for brevity
  and since whole numbers are easier to reason about & work with).
• Changes single-quotes to double-quotes in the <rect> for consistency.
• Moves most <image> attributes to the start and insert some newlines before
  its href/xlink:href attributes for improved readability.
• Removes the unnecessary/unused xmlns:xlink namespace declaration from the
  inner SVG document (inside the image) in the second test.
• Makes the "test()" function wait for a paint before making the dynamic
  attribute-removal (and changes that function to use async/await for
  readability). This makes these tests more robust, to be sure they actually
  have something to invalidate in the first place.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: Bug 1908991 part 2: Add a test that removes xlink:href (rather than href) from an SVG image element. r?longsonr

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Bug 1908991 part 4: Add WPTs to test modifications to a SVG image element's href and xlink:href attributes. r?longsonr

This patch adds each new tests as a modified-copy (with 'hg cp' + edits) of the
correspondingly-numbered "image-remove-href-*" test.

The second test fails for some reason, though it seems to pass in Chrome, so
I'm adding it with an expected-failure annotation for now.

Comment 4

[Daniel Holbert [:dholbert]]

Note that one of the tests in part 3 fails; I've got it annotated as expected-fail for now. I plan to file a followup for that which I can include in the .ini file; toggling ni=me as a reminder. (Going AFK for a bit right now so I can't do it at the moment.)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: Bug 1908991 part 3: Avoid repeated calls to test() in image-remove-href-2.svg. r?longsonr

The test() function is only really designed to run once, but it actually gets
invoked more than once as documented in the new comment. So this patch nerfs
any invocations after the first, to avoid any associated intermittent weirdness
that might result from repeated invocations.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #4)

Note that one of the tests in part 3 fails; I've got it annotated as expected-fail for now. I plan to file a followup for that which I can include in the .ini file; toggling ni=me as a reminder. (Going AFK for a bit right now so I can't do it at the moment.)

I figured out what was going on that was causing that test^ to fail. There were two weird things going on there:
(A) the test was firing its test() function (and modifying/removing its attributes) multiple times, since it had multiple loads.
(B) With those unintentional repeated test() invocations, we happened to trigger a subtle interop difference where removeAttribute('href') does different things in Firefox vs. Chromium/WebKit, depending on whether xlink:href is present and whether it's been modified.

I've addressed (A) by nerfing repeated calls to the load-handler in the tests associated this bug that were susceptible to this (i.e. the tests where the <image> element's load-handler kicks off a new image load internally which results in another call to the load handler). This nerfing makes the test pass now.

I've filed Bug 1909028 to document & hopefully investigate the difference that I described as (B) here.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8e843b44635
part 1: Various tweaks to image-remove-href-* WPT tests. r=longsonr
https://hg.mozilla.org/integration/autoland/rev/9878deb3fb06
part 2: Add a test that removes xlink:href (rather than href) from an SVG image element. r=longsonr
https://hg.mozilla.org/integration/autoland/rev/5dbc434f54bb
part 3: Avoid repeated calls to test() in image-remove-href-2.svg. r=longsonr
https://hg.mozilla.org/integration/autoland/rev/89d7e0466160
part 4: Add WPTs to test modifications to a SVG image element's href and xlink:href attributes. r=longsonr

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/47215 for changes under testing/web-platform/tests

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/e8e843b44635
https://hg.mozilla.org/mozilla-central/rev/9878deb3fb06
https://hg.mozilla.org/mozilla-central/rev/5dbc434f54bb
https://hg.mozilla.org/mozilla-central/rev/89d7e0466160

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot