1781096 - Flickering images on www.star.nesdis.noaa.gov

Status: VERIFIED FIXED

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

Description

[Sky Schubert [:skyschub]]

On the first run of the mentioned page, the images flicker visibly. Subsequent repeats of the animation render without flickering.

The original reporter in the WebCompat bug mentioned this is a regression, and mozregression points its fingers towards bug 560067. Emilio, since this was your patch, could you have a look please?

Comment 1

[Sky Schubert [:skyschub]]

[Tracking Requested - why for this release]:
Preemptively requesting RelMan tracking since this is affecting 102esr. The visual flicker in this case could cause harm to people, but we don't know (yet?) if this issue is isolated to this site or is affecting other sites.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Dennis, which commit of the ones in bug 560067 was the regressing one? It'd be great if you have the changelog that mozregression showed around, since that bug landed in two chunks. Otherwise I can try to mozregression myself tomorrow. Thanks.

Comment 3

[Mayank Bansal]

2022-07-26T08:06:03.014000: DEBUG : Found commit message:
Bug 560067 - Ensure visibility tracking code doesn't choke on visibility:hidden images. r=tnikkel

visibility: hidden still goes into BuildDisplayListForChild (because
children might be visible), but the child itself might not be visible.

Differential Revision: https://phabricator.services.mozilla.com/D122117

2022-07-26T08:06:03.014000: DEBUG : Did not find a branch, checking all integration branches
2022-07-26T08:06:03.015000: INFO : The bisection is done.
2022-07-26T08:06:03.017000: INFO : Stopped

Comment 4

[Sky Schubert [:skyschub]]

Ah, sorry, Emilio. Didn't notice that there was more than one revision. I can confirm what Mayank said.

Mayank, thanks for taking care of that while I was asleep! Much appreciated. :)

Comment 5

[Jonathan Kew [:jfkthame]]

I can confirm the white flicker between frames on first run of the animation is quite bad here. Marking as S2 given it's a visible regression (in an older release, I didn't see any flicker).

Comment 6

[Emilio Cobos Álvarez [:emilio]]

So this seems kinda working as expected, in the sense that they're using:

<div style="visibility: hidden; [snip]" class="cycle-slide">
  <!-- snip -->
  <a href="https://cdn.star.nesdis.noaa.gov/GOES16/ABI/SECTOR/se/Sandwich/20222091006_GOES16-ABI-se-Sandwich-2400x2400.jpg" class="FBAnim" title="Sandwich - Blend combines IR band 13 with visual band 3 - 28 Jul 2022 - 1006 UTC">
    <img id="F_23" src="https://cdn.star.nesdis.noaa.gov/GOES16/ABI/SECTOR/se/Sandwich/20222091006_GOES16-ABI-se-Sandwich-1200x1200.jpg" crossorigin="Anonymous" alt="Sandwich - Blend combines IR band 13 with visual band 3 - 28 Jul 2022 - 1006 UTC" title="Sandwich - Blend combines IR band 13 with visual band 3 - 28 Jul 2022 - 1006 UTC" class="img-responsive" width="1200" height="1200">
  </a>
  <a href="https://cdn.star.nesdis.noaa.gov/GOES16/ABI/SECTOR/se/Sandwich/20222091006_GOES16-ABI-se-Sandwich-2400x2400.jpg" class="FBAnim" title="Sandwich - Blend combines IR band 13 with visual band 3 - 28 Jul 2022 - 1006 UTC">
    <img src="images/latlon/g16_se_grid_1200x1200_yellow.gif" class="OLLatLon img-responsive" alt="Latitude / Longitude grid" style="z-index: 1000;">
  </a>
</div>

Before my change, we would decode all images in the slide show because we considered them visible. After my change we'd only decode them once they become visible.

We decode images async by default (WebKit does sync-decoding by default instead), so there's a flash since they're not using the same image element. The tool we provide sites to prevent this is using decoding="sync" on the image.

We could of course reverse my change, but it'd be a pity since it is really fixing a bug. Dennis, would there be any chance of contacting the site asking to change the images to use decoding="sync"?

Comment 7

[Sky Schubert [:skyschub]]

I sent them an email and asked them for feedback/action. I'll note that for governmental sites, the chance of them responding is... 50/50. I'll update this bug as soon as I have something to update.

Since this is the only regression WebCompat is aware of right now, I think that's a good enough outcome for now. :)

Comment 8

[Sky Schubert [:skyschub]]

Oh! Luckily, it's the good 50% this time! I just received an email back,

Wow, I'm the UI developer who wrote the animation code for the GOES site, and yes, it has come to our attention that particularly since a recent update Firefox, this behavior has been a problem for our FireFox users.

As soon as I finish my breakfast, I'm going to test this fix on our dev site, and will share a link for your review.

So we should be able to get this worked out.

Comment 9

[Sky Schubert [:skyschub]]

I'll unflag this for RelMan tracking, since I'm 99% confident this won't need platform changes.

Comment 10

[Dianna Smith [:diannaS]]

:denschub is there an update on the fix from the site?

Comment 11

[Emilio Cobos Álvarez [:emilio]]

It seems they have decoding="sync" now in the page, but it flickers the first run still... I'll poke when I have some time.

Comment 12

[Emilio Cobos Álvarez [:emilio]]

Ah, so it doesn't flicker in release, but it flickers on Nightly, as a result of bug 1782247 apparently.

So the compat issue is fixed, we just need to take care of the regression here... Repurposing for that.

Comment 13

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1781096 - Fix decoding="sync" when the frame is created after the image attribute. r=aosmond

The fix is the one line in
nsImageLoadingContent::MaybeForceSyncDecoding, but I added new asserts
that should prevent this from regressing in the future.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad5f21f4cdd0
Fix decoding="sync" when the frame is created after the image attribute. r=tnikkel

Comment 15

[Norisz Fay [:noriszfay]]

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

Comment 16

[Ciprian Georgiu, Desktop QA]

The issue described in comment https://github.com/webcompat/web-bugs/issues/106618, is not reproduing anymore on latest Beta 105.0b5 under Win 10 x64.

However, I am able to reproduce the one time flash after accessing https://www.star.nesdis.noaa.gov/GOES/sector_band.php?sat=G16&sector=umv&band=07&length=48 website, the issue filed in https://github.com/webcompat/web-bugs/issues/108010. Was this bug supposed to be fixed by this patch as well, can you please take a look, Emilio? I've tested this on macOS 11 with latest Beta 105.0b6.

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Do you have a screencast? I don't see this on Nightly.

Comment 18

[Ciprian Georgiu, Desktop QA]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)

Do you have a screencast? I don't see this on Nightly.

I've made one, you can check it here. Let me know if I can help with anything else.

Comment 19

[Ciprian Georgiu, Desktop QA]

It seems that the one time flickering can also be reproduced on Chrome. The github report suggested that the issue is only affecting Firefox. Maybe this is a site issue, after all?

Comment 20

[Emilio Cobos Álvarez [:emilio]]

Ah, yeah, that's definitely not what this bug is about, this is about the flickering of the images in the map.

Comment 21

[Ciprian Georgiu, Desktop QA]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #20)

Ah, yeah, that's definitely not what this bug is about, this is about the flickering of the images in the map.

Yes, that's fixed. I thought that the one time flickering from my previous comment, was aslo going to be fixed, since the https://github.com/webcompat/web-bugs/issues/108010 was marked dupe of this bug.

I think we can mark this as verifeid fixed per comment 16.