1469000 - Painting problem with some animated gif when embedded in a page

Status: VERIFIED FIXED

(Core :: Web Painting, defect)

Description

[Julien Wajsberg [:julienw]]

STR:
1. Go to https://css-tricks.com/understanding-web-fonts-getting/#article-header-id-4
2. Look at the animated gif in the paragraph "Spacing and Kerning".

=> The gif isn't painted properly

Note it works fine with displaying the gif directly: https://res.cloudinary.com/css-tricks/image/upload/c_scale,w_1000,f_auto,q_auto/v1518304652/kerning-explainer_bfxl5k.gif

This works in Chrome.

The same happens in https://css-tricks.com/tag/variable-fonts/ with the image right before "A simple resource for finding and trying variable fonts". What's interesting here is that this is a link and we seem to force a correct paint when the user hovers it.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: about:support dump

Comment 2

[Julien Wajsberg [:julienw]]

This works fine in beta 61.

Comment 3

[Julien Wajsberg [:julienw]]

regression gave me https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b479a3309609c69098ab05bf46e0e6c0b04d2a4e&tochange=f1901407d4fae48934787c62dcb699f75ad0c60d

Emilio you're the possible culprit here ;) Could you have a look?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

There's no way that patch changes that, my patch just avoids going through FFI to get an atom... The gif works for me, is there any chance you could print the previous regression range?

Comment 5

[Jeremie Patonnier :Jeremie]

Confirm on Mac OSX with:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:62.0) Gecko/20100101 Firefox/62.0

Comment 6

[Julien Wajsberg [:julienw]]

Hi Emilio,

I did the mozregression again and indeed found another result, that looks more appropriate:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d7c126843dc0b7cc7251bbf7c21c954d3f391c49&tochange=fc4ca25a0f3941b251ff1a8424e15c6d95c848c9

I must have screwed up my previous run.

You're still the designated author though ;-)

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Dang, alright, that does sound more feasible, I'll take a look. Thanks!

I still can't repro the bug on Linux though... Given that image does use a viewport-dependent sizes attribute, I think it's way more likely to be bug 1149357's fault.

Comment 8

[Julien Wajsberg [:julienw]]

(Ah, I don't know if this is bug 1462272 or bug 1149357)

Comment 9

[Julien Wajsberg [:julienw]]

(oops sorry for the noise)

> I still can't repro the bug on Linux though...

I'm myself on Linux (Debian stable). Using an Intel GPU in case this is some useful information...

Comment 10

[Julien Wajsberg [:julienw]]

Still happening in latest nightly, buildid 20180619102337, Linux.

Comment 11

[Jeremie Patonnier :Jeremie]

Sorry, I missed my buildid but it's the same as Julien: 20180619102337 on MacOS

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Alright, so I managed to repro it, but not with WR enabled, can anybody kindly confirm this? :)

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced testcase.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Taking for now for investigation, not sure if my patches introduce or just expose an existing bug.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Ok, I understand what goes wrong now.

Comment 16

[Julien Wajsberg [:julienw]]

> Alright, so I managed to repro it, but not with WR enabled, can anybody kindly confirm this? :)

Indeed, I don't use WR as you can see in my attached dump from about:support :)

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1469000: Fix image to layout transform for invalidation.

When computing image to layout transforms for invalidation, use the actual
intrinsic size of the source image, instead of the layout intrinsic size, which
may be scaled by ResponsiveImageSelector.

I have absolutely no idea how to write a test for this, suggestions welcome.

Review commit: https://reviewboard.mozilla.org/r/251632/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/251632/

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8986219 [details]
Bug 1469000: Fix image to layout transform for invalidation.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/251632/diff/1-2/

Comment 20

[Emilio Cobos Álvarez (:emilio)]

I looked into delaytest.html to test this, but looks like it'll bring pain... Suggestions welcome.

Comment 21

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8986219 [details]
Bug 1469000: Fix image to layout transform for invalidation.

https://reviewboard.mozilla.org/r/251632/#review259796

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking so that we will take a look at this for uplift to beta 62.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12a849d78301
Fix image to layout transform for invalidation. r=aosmond

Comment 24

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/12a849d78301

Comment 25

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8986219 [details]
Bug 1469000: Fix image to layout transform for invalidation.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1149357 
[User impact if declined]: bogus repaint / clip on some images, see this bug and bug  1469784 
[Is this code covered by automated tests?]: Unfortunately not, see bug 1470350
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: Load this test-case or the website in the other bug.
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: It's a simple fix to the image size we invalidate to, effectively reverting to the same behavior before bug 1149357
[String changes made/needed]: none

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Did someone verify this in Nightly? Maybe in one of the other bugs?

Comment 27

[Jens]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #26)
> Did someone verify this in Nightly? Maybe in one of the other bugs?

FWIW, I am the reporter of duplicate bug 1469784 and I can verify that at least for the duplicate it is fixed.

Comment 28

[Emilio Cobos Álvarez (:emilio)]

Yes, I also verified, and you can see that the gif in https://bugzilla.mozilla.org/attachment.cgi?id=8986181 displays correctly now.

Comment 29

[Julien Wajsberg [:julienw]]

The original URL is working fine now as well.

Comment 30

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8986219 [details]
Bug 1469000: Fix image to layout transform for invalidation.

Verified in nightly, fix for a regression in 62; let's uplift for beta 8.

Comment 31

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/80347cdf232e

Comment 32

[Stefan [:StefanG_QA]]

I was able to reproduce this issue using Nightly build from 20180621100048 on Windows 10, OS X 10.14 and Ubuntu 18.04.

I have verified that the issue is no longer reproducible on the latest Nightly build (63.0a1, 20180711100118) on Windows 10 x64, Mac 10.14 and Ubuntu 18.04 x64. 
Based on this and comments 28&29 I will mark the issue as Verified-Fixed on Firefox63.

Comment 33

[Stefan [:StefanG_QA]]

I was able to reproduce this issue using Beta 62.0b7 on Windows 10, OS X 10.14 and Ubuntu 18.04.

I have verified that the issue is no longer reproducible on the Beta 62.0b8 on Windows 10 x64, Mac 10.14 and Ubuntu 18.04 x64. 
Based on this, I will mark the issue as Verified-Fixed on Firefox62.