1554664 - border-radius on <img> tag doesn't always load initially

Status: VERIFIED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[teuneboon]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

1. Made an image with a border-radius of 50% like so: <img src="https://www.gravatar.com/avatar/9916c685fb33f3692cca815527058d34?s=32&amp;d=mm" style="border-radius: 50%;">
2. Load the page this image is on

Actual results:

Image is shown, but doesn't have a border radius, it's just a normal square. Once I try to use inspect element on it, it suddenly has a border radius.

It's seemingly randomly reproducible on: https://jsfiddle.net/jzotx46v/1/. Sometimes it initially seems to load with border radius there, and sometimes it's just a square(pressing the "Run" button resets it to being square for me). It's always reproducible if I make a new tab "about:blank" and use the developer tools to just inject the problematic HTML in there.

Expected results:

Image should have a border radius

Comment 1

[Alice0775 White]

I can reproduce the issue on Nightly69.0a1 Windows10.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=94682e23ba118dcfab3843a3280618b2e1686ecb&tochange=fdc7b66d76a4ded62d0292bef4de0ffb090d9587

Regressed by: Bug 1395964

:emilio
Your patch seems to cause the regression. Can you please look into this?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

[Tracking Requested - why for this release]: Somewhat bad regression.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: test-case that shows the underlying issue.

This is really an issue with bug 1367906.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1554664 - Work-around broken border-radius caching. r=mstange,mattwoodrow

This is just a super-low-risk work-around for bug 1554755 that we can uplift to
beta, I'll follow-up in a bit in there with a fix and a test.

The reason my patch breaks this is because my patch stopped making images
reframe.

The reframe was of course a very expensive way of invalidating that bit ;)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0765f0fa7560
Work-around broken border-radius caching. r=mattwoodrow

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9067853 [details]
Bug 1554664 - Work-around broken border-radius caching. r=mstange,mattwoodrow

Beta/Release Uplift Approval Request

• User impact if declined: Some images won't get clipped by border-radius after loading.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see comment 0, basically just open data:text/html,<img src="https://www.gravatar.com/avatar/9916c685fb33f3692cca815527058d34?s=32&amp;d=mm" style="border-radius: 50%;">
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Ultra-low-risk patch intentionally, to be able to uplift to beta. An optimization introduced by bug 1395964 uncovered a pre-existing, londstanding bug (which I filed and proposed a patch for, see bug 1554755).

The patch just literally makes us re-compute the border radius next time we're asked for it after an image load. So I think it's basically zero-risk.

• String changes made/needed: none

Comment 7

[Emilio Cobos Álvarez (:emilio)]

A quick quote from IRC, just to better explain what's going on:

00:30 <mattwoodrow> emilio: I don't really understand the correlation between image frames getting a new image, and border-radius
00:32 <@emilio> mattwoodrow: we initially (before the load) lay out the image with 0x0 size, which makes GetBorderRadii compute to zero (since all the radii are 50%)
00:32 <@emilio> mattwoodrow: the image load changes the size of the image, but not the style, which means that the mMayHaveRoundedCorners bit gets out of sync
00:32 <mattwoodrow> emilio: Ah right, so it's ultimately the size change on the frame that matters, image load is just a cause of that?
00:33 <mattwoodrow> Can other frame types have similar issues?
00:33 <@emilio> mattwoodrow: right. The image load used to cause a reframe itself before my patch.
00:33 <@emilio> mattwoodrow: yeah, this is independent of the frame type, see bug 1554755

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 9067853 [details]
Bug 1554664 - Work-around broken border-radius caching. r=mstange,mattwoodrow

regression fix, approved for 68.0b6

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/0765f0fa7560

Comment 10

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0a91cbfe3b3f

Comment 11

[Timea Zsoldos [:tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 69.0a1 (2019.05.27) on Win 10 x64.
I can confirm this issue is fixed, I verified using Firefox 69.0a1 (2019.05.29) Win 10 x64, Ubuntu 16.04 x64 and macOS 10.13.6, waiting for fix in beta 68.0b6.

Comment 12

[Timea Zsoldos [:tzsoldos], Desktop QA]

I can confirm this issue is fixed, I verified using Firefox 68.0b6 on Win 10 x64, Ubuntu 16.04 x64 and macOS 10.13.6.