1866911 - Checkmark is missing on Fidelity NetBenefits

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Visit https://nb.fidelity.com/static/mybenefits/netbenefitslogin/#/login
2. Click the "Remember my username" checkbox, to make it turn green.

EXPECTED RESULTS:
It should have a white checkmark in the green area.

ACTUAL RESULTS:
The box just turns green (no white chekcmark)

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6cece559246d50b5c9af6c675e06ad4da2a68fc7&tochange=3f0a7131d0fd0bf13d36e723db4d1f84bbd2291e

--> Regressed by bug 1806964 , though maybe this is to-be-expected if this is just fidelity using the feature that we're locking down here.

On my local machine, Chrome 121.0.6129.0 (Official Build) dev (64-bit) gives EXPECTED RESULTS for now, though per bug 1806964 comment 2, maybe they just haven't shipped this behavior yet. This version of Chrome does show me an orange rect alongside the green one when I view the WPT from bug 1806964

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot (Firefox Nightly 122 on top, Chrome 121 dev on bottom)

Comment 2

[Daniel Holbert [:dholbert]]

Hmm, the pref svg.use-element.data-url-href.allowed doesn't seem to help here. I still see ACTUAL RESULTS after toggling that pref to true (even after restarting Firefox for good measure).

And looking in devtools, it looks like this isn't <use> with a data URI at all...

Instead it looks like this, inside of a <s-root> custom-element:

<svg aria-hidden="true" class="pvd-icon-root pvd-checkbox__checkbox-wrap-icon pvd-checkbox__checkbox-wrap-icon--checked pvd-icon--extra-small" focusable="false">
      <!--?lit$1723902413$-->
      <use href="#pvd-action__check" xlink:href="#pvd-action__check"></use>
    </svg>

...which references this definition elsewhere in the same document:

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
  <symbol id="pvd-action__check" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
    <path d="M2.24 10.86A1.25 1.25 0 10.27 12.4l6.33 8a1.25 1.25 0 001.87.1L23.64 5.28a1.25 1.25 0 10-1.78-1.77L7.68 17.72l-5.44-6.87z"></path>
  </symbol>

So: RE my "maybe this is to-be-expected" in comment 0, it looks like something's off here, since the pref isn't working and this doesn't seem to be a case where we wanted behavior to change.

Comment 3

[Daniel Holbert [:dholbert]]

Tom could you take a look and see how/why we're falling over here? I wonder if this is something special about the fact that we're in a custom element, referencing a definition outside of that element; maybe that situation is falling out differently from the old vs. new logic here.

Comment 4

[Daniel Holbert [:dholbert]]

(Triaging as S2, since this looks like unintended fallout which could cause real breakage. We should fix this regression before it hits release.)

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1806964

Comment 6

[intrnl]

Another site with the same issue, though it's not within a custom element
https://intrnl.github.io/ngs-world-time/

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox122 (nightly). However, the bug still isn't assigned.

:fgriffith, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to intrnl from comment #6)

Another site with the same issue, though it's not within a custom element
https://intrnl.github.io/ngs-world-time/

Interesting! Thanks for that testcase.

There seems to be some sort of race condition or statefulness here, which makes this not reproduce (so far) in a standalone testcase taken from these examples.

One thing I'm noticing: when loading either of the affected sites here, you can "fix" the issue by right-clicking the use element in devtools and choosing "Edit as SVG", and then make any non-functional edit (e.g. insert a single space character before the element). This makes the graphic immediately show up.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

This is clearly wrong, the original document could be ~anywhere.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1866911 - Add a test for this bug. r=longsonr,dholbert,evilpie,#layout

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab3a739ad0ee
Don't use mOriginal to do the clone element lookup. r=dholbert

Comment 13

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

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

Comment 14

[Cristian Tuns]

Backed out for causing reftest failures in use-01-extref.svg

• Backout link
• Push with failures
• Failure Log
• Failure line: REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/svg/use-01-extref.svg == layout/reftests/svg/pass.svg | image comparison, max difference: 255, number of differing pixels: 304000

Comment 15

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

Upstream PR was closed without merging

Comment 16

[Robert Longson [:longsonr]]

I suspect we aren't going to be able to use ResetWithID at all here. The line you removed was added specifically to pass use-01-extref.svg

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Okay so I asked for a backout of the regressor, but then thinking more about it I think I have a reasonable fix...

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e5d22339af44
Add a test for this bug. r=dholbert

Comment 19

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/e5d22339af44

Comment 20

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

Upstream PR merged by moz-wptsync-bot

Comment 21

[Daniel Holbert [:dholbert]]

Verified fixed in current nightly 122.0a1 (2023-12-05) (just tested)

Comment 22

[Daniel Bodea [:danibodea]]

This issue was reproduced in Nightly v122.0a1 from 2023-11-28 and verified in Nightly v123.0a1 and Beta v122.0b2 on Windows 10, MacOS 11 and Ubuntu 22.