Use img tag instead of picture #2067
Conversation
|
I welcome a closer look at (all) imagery! 👏 Specifically,
as long as this replacement provides parity support for higher resolution when one uses a device with higher density screen (or zooms in low-density desktop over 133%) and we can still control distinctive “steps” of HiDPI support generically by manipulating fastly’s My (poor!) understanding regarding
is that our use of |
|
cc @sndrs coz he was the author of initial implementation and ads are boring… |
|
Related (beyond parity): guardian/frontend#12216 (comment). |
|
Size Change: -37 B (0%) Total Size: 716 kB
ℹ️ View Unchanged
|
|
For reference, here is why we moved from img -> picture in Frontend: guardian/frontend#11400 |
Oh! Good find, I forgot completely. Makes sense. We don’t want a super-duper-highres phone to willy nilly pick up highest res source just because we provide one for half-super-duper desktops. |
And here's why we moved from |
This is interesting. The original reason we migrated from
It was found that this provided the best balance of performance/bandwidth/noticeable image quality. How does this change manage flipping between the two |
|
Closing this in favour of |
|
Ressurecting as a potential stopgap ahead of |
oliverlloyd
requested review from
gtrufitt,
jlieb10,
liywjl and
shtukas
as code owners
|
@paperboyo @sndrs @JamieB-gu We're thinking of moving ahead with this change as a stopgap ahead of migrating to |
|
This is great! It will be such a pleasure to finally see good quality images on DCR! Do you think it may make sense to spell out in the PR description the two reasons we still think it makes sense to move to Thank you for the examples, this is very useful, one could almost imagine automated tests that would compare CSS widths (and one day heights too) of images at different breakpoints with actual requests made on certain devices. They should be able to help find inconsistencies and mistakes, like eg iphonex dpr3: Main media which looks mad and wrong, but then you notice it’s a Photo essay – and everything suddenly makes sense, haha. |
I've added to the description to make it clear what the future goals are |
This would be a great contract to define. Eg.: If I have this device, at this screen width, and pass these srcsets, with this role, then I expect |
Okay, so I don't think this is realistically possible. We want to test that various browsers, on different devices, at different breakpoints, do what we expect. But test automation in this field is typically done headless, where the presentation part of the browser is removed and the focus is on the html. We could (and do) use Storybook to render images at various breakpoints but we still can't assert these to check that the right image was picked. Sadly, unless someone else has a better idea or wants to write some nifty pupetter script or something, I think this process will continue to be a manual. This PR lists a good baseline for this. |
Absolutely fine and kudos for having a look! In my naive mind I imagined two robots: first robot that builds |
|
👏 Most popular inline images of 620px seem to be requested wrongly at 445px. But DPR 2 :-). |
I'm not able to replicate this; I'm getting the 620px, dp2 image everytime, on DCR, Frontend, same same. Is it still happening? |
Very, very weird. Chrome OK. Firefox (Mac and Win, different versions) OK in Private Browsing, but not in my main session (cleared site data, no extensions that aren’t also in Private, same zoom):
Probably something on my system, apologies, will report back if I find out what or repro somewhere else… |
|
Is this because the browser is favouring a cached img that it has locally over making a network request? I think the spec talks about this functionality. |
|
EDIT: disregard the disregarding: this third machine was a genuine retina Mac (dpr 2) |
It's nice to see this happening in the wild. @paperboyo you can test this theory using the "Disable cache" checkbox in the Network tab of the Firefox devtools (it will only disable it while the tools are open AFAIK). |
|
Yeah, I disabled the cache. Done some more tests. My current operating hypothesis is that Firefox seems to be choosing sources differently from this ( As I’m permanently zoomed-in to 133% (dpr 1 device) on the Guardian, unfortunately I’m getting smaller images than on frontend. But:
*correct means: when zoomed-in over 130% (or whatever media query specifies), on a dpr 1 device, request an image of CSS pixels width (strangely, this is the broken part, I’m getting 445px requests for inline article images of 620 CSS pixels!) at dpr 2. |
If you drop into responsive design mode on Firefox (Cmd-Opt-M) there's a dropdown to simulate different DPRs, don't know if that's helpful? |
|
Yeah, yeah, I know. But it doesn’t accept 1.3 which is what I would need to drop it down to on this machine I’m currently sitting on (drp 2) to simulate my home machines (both PC and Mac of dpr 1) being zoomed in over 130%. Anyway :-). Can anyone with a Firefox on dpr 1 desktop can confirm 620px images are requested at 445px? (yes, I know, dpr 2, but still: too small dpr 2) |
What?
This PR proposes replacing the picture tag with an img tag as a stopgap ahead of using image-rendering
TLDR
Surprisingly, tests for this change showed a lot of deviation between Frontend vs. DCR, both DCR with a picture tag and DCR with an img tab. More surprisingly, DCR with an img tag often outperformed Frontend. These variations could be for a lot of reasons though, not purely because an
imgtag was used.How?
There are two main code changes here:
Picture(now calledImg) componenthdipirelated logic and adding the ability to create sizes.Why?
DCR has known issues where it can sometimes render substantially lower quality images than appropriate in the context of position, screen width and resolution. In the short term, this change brings us into line with Frontend in most cases, makes us actually do better than frontend in some cases, and leaves us at the same or an acceptable level below frontend in the rest.
Why is this a stopgap?
See this ongoing discussion for more detail on our medium to longer term goals. But to summarise we want to:
Solve the Wealthy Guy problem. Mobile devices with small screens but high dpr values can end up being served excessively large files, wasting bandwidth.
Finesse the quality. We want to be able to specify a different set of images to use above a certain resolution. This is because we've found that for high resolution screens, by using
dpr=2&quality=45together we can reduce the file size without noticeably impacting quality.Both of these need a
picturetag based solution.Test methodology
I rendered different images using a variety of simulated devices (via Firefox and Chrome dev tools) and then checked the resulting src chosen by the browser. Test were done using code from Frontend, new DCR and current DCR.
Example urls:
https://www.theguardian.com/artanddesign/2020/may/11/cross-border-love-a-photo-essay?dcr=false
http://localhost:3030/Article?url=https://www.theguardian.com/artanddesign/2020/may/11/cross-border-love-a-photo-essay
https://www.theguardian.com/artanddesign/2020/may/11/cross-border-love-a-photo-essay?dcr=true
Results
Desktop DPR 2
immersive
Frontend:
width=1300|quality=45|dpr=2New DCR:
width=1300|quality=45|dpr=2Current DCR:
width=1300|quality=85showcase (main media 1020 pixels)
Frontend:
width=1300|quality=45|dpr=2New DCR:
width=1020|quality=45|dpr=2Current DCR:
width=1020|quality=85inline
Frontend:
width=620|quality=45|dpr=2New DCR:
width=620|quality=45|dpr=2Current DCR:
width=620|quality=85showcase (860 pixels wide)
Frontend:
width=880|quality=45|dpr=2New DCR:
width=940|quality=45|dpr=2Current DCR:
width=1020|quality=85thumbnail
Frontend:
width=140|quality=45|dpr=2New DCR:
width=140|quality=45|dpr=2Current DCR:
width=140|quality=85Desktop DPR 1
inline
Frontend:
width=620|quality=85New DCR:
width=620|quality=85Current DCR:
width=620|quality=85iphone (375 pixels wide)
inline
Frontend:
width=300|quality=85New DCR:
width=445|quality=85Current DCR:
width=445|quality=85immersive
Frontend:
width=300|quality=85New DCR:
width=465|quality=85Current DCR:
width=445|quality=85thumbnail (140 pixels)
Frontend:
width=300|quality=85New DCR:
width=140|quality=45|dpr=2Current DCR:
width=445|quality=85iphonex dpr3
main media
Frontend:
width=2000|quality=85New DCR:
width=1125|quality=85Current DCR:
width=445|quality=85inline
Frontend:
width=445|quality=45|dpr=2New DCR:
width=605|quality=45|dpr=2Current DCR:
width=445|quality=85galaxy s9 dpr4
main media
Frontend:
width=2250|quality=85New DCR:
width=725|quality=45|dpr=2Current DCR:
width=445|quality=85inline
Frontend:
width=445|quality=45|dpr=2New DCR:
width=620|quality=45|dpr=2Current DCR:
width=445|quality=85immersive
Frontend:
width=465|quality=45|dpr=2New DCR:
width=725|quality=45|dpr=2Current DCR:
width=445|quality=85