Closed Bug 1758975 Opened 2 years ago Closed 2 years ago

GIF without global color table allows seeing through Firefox window on Linux

Categories

(Core :: Graphics: WebRender, defect, P1)

Firefox 100
x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
100 Branch
Tracking Status
firefox-esr91 --- verified
firefox98 --- wontfix
firefox99 --- verified
firefox100 --- verified

People

(Reporter: mail, Assigned: aosmond)

References

(Blocks 1 open bug)

Details

(Keywords: correctness, regression)

Attachments

(3 files)

Attached image Screenshot

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

Steps to reproduce:

Tested using Firefox 97.0.2 and Nightly 100.0a1, both on Linux
View this GIF: https://blog.darrien.dev/you-dont-know-gif/Sunflower_as_gif_89a-no-gct.gif
(GIF is from this blog post https://blog.darrien.dev/posts/you-dont-know-gif/, bug noticed on HackerNews https://news.ycombinator.com/item?id=30626502, people reported the same bug using both X and Wayland)

Actual results:

Where the GIF should appear instead is visible whatever is behind that part of the Firefox window, e.g. my desktop background.

Expected results:

Display the GIF as a solid black rectangle.

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Widget: Gtk
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Has STR: --- → yes
Ever confirmed: true
Keywords: correctness
OS: Unspecified → Linux
Hardware: Unspecified → x86_64

Mozregression says: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=404680ad41e4e7d08aa46081a70095486f14e15f&tochange=e704e3565db9260efaa854f2916ca023b717db3c

That's when WebRender was enabled by default for my hardware (confirmed that the good build had "Compositing: Basic", and the second one had WebRender enabled).

I wonder if we think the image is opaque (and thus avoid putting it in the alpha batches) but it's not, causing this funny effect.

Component: Widget: Gtk → Graphics: WebRender

Yeah, confirmed that this makes the artifact not happen (we just render a fully-transparent image). Note that Chrome and Firefox pre-webrender draw black:

diff --git a/gfx/webrender_bindings/src/bindings.rs b/gfx/webrender_bindings/src/bindings.rs
index ffcbe988c2ed3..40fc2b6881a12 100644
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -338,7 +338,7 @@ impl<'a> From<&'a WrImageDescriptor> for ImageDescriptor {
         let mut flags = ImageDescriptorFlags::empty();
 
         if desc.opacity == OpacityType::Opaque {
-            flags |= ImageDescriptorFlags::IS_OPAQUE;
+            // flags |= ImageDescriptorFlags::IS_OPAQUE;
         }
 
         ImageDescriptor {

So probably an imagelib bug?

Gnome Xwayland, Debian Testing, Intel
good: black
bad: transparent
MOZ_DISABLE_CONTENT_SANDBOX=1 mozregression --good 2018-01-01 --bad 2019-01-01 --pref gfx.webrender.all:true layers.acceleration.force-enabled:true gfx.webrender.enabled:true gfx.webrendest.enabled:true -a https://bugzilla.mozilla.org/attachment.cgi?id=9267224

4:27.53 INFO: Last good revision: 5165e750ffabb930deb846410ab62dcc6d1f9e52 (2018-09-16)
4:27.53 INFO: First bad revision: 87a95e1b7ec691bef7b938e722fe1b01cce68664 (2018-09-17)
4:27.53 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5165e750ffabb930deb846410ab62dcc6d1f9e52&tochange=87a95e1b7ec691bef7b938e722fe1b01cce68664

Has Regression Range: --- → yes
Keywords: regression

14c5eab0c6b359b7904a8d9807c8282352707ba0 Martin Stransky — Bug 1489097 - [Linux/Gtk] Enable default ARGB visual for toplevel windows on GNOME, r=jhorak

because it's reproducible with last good and mozilla.widget.use-argb-visuals:true
MOZ_DISABLE_CONTENT_SANDBOX=1 mozregression --launch 2018-09-16 --pref gfx.webrender.all:true layers.acceleration.force-enabled:true gfx.webrender.enabled:true gfx.webrendest.enabled:true mozilla.widget.use-argb-visuals:true -a https://bugzilla.mozilla.org/attachment.cgi?id=9267224

Regressed by: 1489097

:stransky, since you are the author of the regressor, bug 1489097, could you take a look?
For more information, please visit auto_nag documentation.

Flags: needinfo?(stransky)

going back with mozilla.widget.use-argb-visuals:true:
2017-10-01 = grey (browser background)
2017-12-28 = transparent

MOZ_DISABLE_CONTENT_SANDBOX=1 mozregression --good 2017-10-01 --bad 2017-12-28 --pref gfx.webrender.all:true layers.acceleration.force-enabled:true gfx.webrender.enabled:true gfx.webrendest.enabled:true mozilla.widget.use-argb-visuals:true -a https://bugzilla.mozilla.org/attachment.cgi?id=9267224

4:00.74 INFO: Last good revision: a77c628829b389ed6ac608eadc88b09eb5115bef (2017-11-17)
4:00.74 INFO: First bad revision: 3a0aac55195fd50ca4b1b41be450bfbaaafc191c (2017-11-18)
4:00.74 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a77c628829b389ed6ac608eadc88b09eb5115bef&tochange=3a0aac55195fd50ca4b1b41be450bfbaaafc191c

contains WR image and gtk changes

Severity: -- → S2
Priority: -- → P1

Pretty sure bug 1489097 only changed behavior here because it only effectively changes what happens when we don't draw to the frame buffer. The bug is in WR.

No longer blocks: wr-correctness
Flags: needinfo?(stransky)
No longer regressed by: 1489097

Glenn, is this already on your radar? Seems fairly severe.

Flags: needinfo?(gwatson)

We've seen this happen when an image is marked as opaque / BGRX, but actually has transparency. This is probably imagelib misreporting. I see my major refactoring for integrating WR with ImageLib from 2017 is in the push log.

Flags: needinfo?(mail)
Flags: needinfo?(mail) → needinfo?(aosmond)

Chrome and Safari think the image is opaque, and our gif decoder does too, so it tags it that way. But we write all 0 (transparent black) pixels. Is imagelib supposed to write FF for alpha in this situation?

Flags: needinfo?(gwatson)
Assignee: nobody → aosmond
Pushed by aosmond@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0938f086f24
Ensure GIF color tables are opaque by default. r=tnikkel
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch

Comment on attachment 9267973 [details]
Bug 1758975 - Ensure GIF color tables are opaque by default.

Beta/Release Uplift Approval Request

  • User impact if declined: Carefully crafted GIFs may cause us to push transparent data marked as opaque to the compositor causing rendering glitches
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): All we do is clear the global/local color tables. Most GIFs will populate these tables with their actual values and there is no change. The patch is very simple and easily understood.
  • String changes made/needed:
Flags: needinfo?(aosmond)
Attachment #9267973 - Flags: approval-mozilla-beta?

Comment on attachment 9267973 [details]
Bug 1758975 - Ensure GIF color tables are opaque by default.

Approved for 99.0b7. Thanks.

Attachment #9267973 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Seems like we should probably take this on ESR also? The patch grafts cleanly there as well.

Flags: needinfo?(aosmond)

Comment on attachment 9267973 [details]
Bug 1758975 - Ensure GIF color tables are opaque by default.

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined: Carefully crafted GIFs may cause us to push transparent data marked as opaque to the compositor causing rendering glitches
  • Fix Landed on Version: 100
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): All we do is clear the global/local color tables. Most GIFs will populate these tables with their actual values and there is no change. The patch is very simple and easily understood.
Flags: needinfo?(aosmond)
Attachment #9267973 - Flags: approval-mozilla-esr91?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Comment on attachment 9267973 [details]
Bug 1758975 - Ensure GIF color tables are opaque by default.

Approved for 91.8esr.

Attachment #9267973 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+

I have managed to reproduce this issue on Ubuntu 22 (wayland) and Nightly v100.0a1 from 2022-03-17. The GIF shows what's behind the browser window, along with some weird light stuttering.

On the latest version of Nightly v100.0a1 from 2022-03-24, it no longer shows what's behind the browser window and neither does it show any stuttering, but the rectangle appears as white, not black, as mentioned in comment 0.

Is this an acceptable result to verify this fix?

Flags: needinfo?(aosmond)

I think the colour doesn't matter too much given it is a corrupted GIF situation, just as long as we do something sensible.

Flags: needinfo?(aosmond)

(To be clear, I expect white.)

This fix was also verified in Beta v99.0b7. There is no build for 91.8esr, yet.
Not even one downloadable "target" file from treeherder:
https://treeherder.mozilla.org/jobs?repo=mozilla-esr91&revision=160f7156fea4465a99bbd3ec66d50bcc9cec4a91&selectedTaskRun=HqlMlbuFSn2H-51-H392gQ.0

Verification for ESR channel is postponed; leaving qe-verify+ flag.

This fix was also verified on ESR v91.8.0esr on Ubuntu 21.10.

QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: