1604886 - Image is unreadably small at Vanguard

Status: RESOLVED WORKSFORME

(Web Compatibility :: Site Reports, defect, P3)

Description

[Daniel Holbert [:dholbert]]

Attachment: screenshot

STR:

1. Visit https://investor.vanguard.com/529-plan/age-based-options
2. Look at the image below the first few explanatory paragraphs. (The image contains some colorful blocks, and the words "stocks / bonds / short-term reserves")
3. (optional) Shift+reload if you don't see anything wrong. You might need to do this a few times.

ACTUAL RESULTS:
The image is extremely small. Its text is unreadable.

EXPECTED RESULTS:
The image should be large enough for its text to read.

The image's size comes from an inline style attribute, set via JS. So the page's JS is deciding that the image should be small, via some measurement while the page is loading, for some reason.

Notes:

• This isn't reproducible in Chrome.
• This isn't reproducible in Nightly 2019-01-01.
• The issue seems to go away on a "light" reload (Ctrl+R), so I suspect it has to do with measurements that are happening while the image is still being downloaded.
• I'm guessing this might be related to our recent image aspect-ratio changes, though that's just a guess.

emilio, maybe you could take a look if you can reproduce & have some spare cycles?

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Happy to take a look. I could reproduce it, but I could also reproduce in 2019-01-01.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

This is the regression range I got, but it could be wrong because nothing there particularly stands out: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b4aeb99d1cb601e5a5288ca05630913fa8528a1c&tochange=b99844d179cacf74a5d39ad23429be91e989c331

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Relevant code from their website, beautified a bit.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Where getElementBounds is effectively getBoundingClientRect(). hasHighResDisplay is window.devicePixelRatio > 1.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Err

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Also, look at this pearl that they do on load which causes us to reconstruct the whole document twice: function sizeLayersOnLoad(){$('html, body').addClass(noScrollClass);scope.sizeLayer();$('html, body').removeClass(noScrollClass); :(

Comment 7

[Emilio Cobos Álvarez [:emilio]]

So I think this is invalid, as in, what's happening is that the load event fires twice in Firefox.

That doesn't seem unreasonable to me but that has the consequence of shrinking the element by 4.

The code does something like:

// While the image is loading, or has loaded but the load event hasn't fired yet.
let src = "highres.png";
if (img.src != src) {
  img.onload = function() {
    // shrink image by half.
  };
  img.src = src;
}

So if we fire the event for the old image and the new one (which can happen, because we don't cancel the old request if it has a size, see this), then we get the shrinking behavior.

In terms of fixing their code, they could clear img.style.width and img.style.height before measuring. That way they'll get the measurement right for the second load event.

Cc'ing Boris in case he has thoughts, or knows if other browsers cancel their requests unconditionally if incomplete or something, but I think this is not a Firefox bug.

Comment 8

[Boris Zbarsky [:bzbarsky]]

or knows if other browsers cancel their requests unconditionally if incomplete or something

I don't know. I think sorting out why this is not an issue in other browsers is a really good idea; then we can decide what to do about it.

It might make some sense to not fire a load event for a just-completing current request if we already have a pending request that might replace it. We could delay that load event until either the pending request replaces it (in which case we should not fire the load event for the thing being replaced) or the pending request is dropped (in which case load should fire).

Comment 9

[Boris Zbarsky [:bzbarsky]]

Actually, I guess if the pending request errors out or is canceled it will fire an error event anyway. So maybe we should just suppress the load event for a current request if there is a pending request? This would need a corresponding spec change, of course... Again, we should check exactly what other UAs do.

Comment 10

[Oana Arbuzov [:oanaarbuzov]]

I'm still able to reproduce the issue on one of my devices on first access and on the second one after 3 page reloads.
https://prnt.sc/10i3fi2

Tested with:
Browser / Version: Firefox Nightly 210309 (🦎 88.0a1-20210302034602)
Operating System: Samsung Galaxy S8 (Android 9) - 1440 x 2960 pixels, 18.5:9 ratio (~570 ppi density), Huawei P20 Lite (Android 8.0.0) - 1080 x 2280 pixels, 19:9 ratio (~432 ppi density)

Comment 11

[Raul Bucata]

It seems that the UI of the page has been updated, and the section is not present.

Daniel, can you confirm this, please?

Tested with:

Browser / Version: Firefox Nightly 102.0a1 (2022-05-05) (64-bit) /Chrome Version Version 101.0.4951.54 (Official Build) (64-bit)
Operating System: Windows 10 PRO x64

Comment 12

[Daniel Holbert [:dholbert]]

Thanks! Yup, that seems to be correct. --> WORKSFORME