1249604 - [Linux] Broken icon shadow during customising process

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Vasilica Tamas, QA [:vtamas]]

[Affected versions]:
Firefox 46.0a2 (2016-02-18)
Firefox 47.0a1 (2016-02-18)

[Affected platforms]:
Ubuntu 12.04 64-bit


[Steps to reproduce]:
1.Launch Firefox with clean profile.
2.Click menu [≡] → select "Customize".
3.Drag a few items from toolbar and drop them into the panel menu.
4.Pay attention to the icon shadow which appears during dragging process.

[Expected Results]:
The icon shadow is correctly displayed during dragging process.

[Actual Results]:
[1] Dragging an item from toolbar to Panel Menu displays an empty rectangle instead the icon shadow.
See screenshot: http://i.imgur.com/oEl1B4Z.png
[2] Dragging an item from Panel Menu to toolbar displays a blurry icon with an unreadable name.
See screenshot: http://i.imgur.com/THiovFA.png


[Regression range]:
(m-c)
Last good revision: 33dc8a83cfc0 (2015-07-28)
First bad revision: 2ee9895e032c (2015-07-29)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=33dc8a83cfc0&tochange=2ee9895e032c

(m-i)
Last good revision: 82f4acb95b5052a779296e0987368c703da7517e
First bad revision: 570c40c6fe4b7059f80c1c681ed0f966bfe382b5
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=82f4acb95b5052a779296e0987368c703da7517e&tochange=570c40c6fe4b7059f80c1c681ed0f966bfe382b5

I suspect that this issue was regressed by Bug 1188138

[Additional notes]:
Firefox 44 and Firefox 45 is not affected because Gtk 3 is disabled across these platforms.

Comment 1

[:Gijs (he/him)]

Looks like a regression from bug 1188138 or bug 1186661, yes. Andrew, can you take a look, please?

Comment 2

[Andrew Comminos [:acomminos]]

Sure. I can't seem to reproduce this locally with 47.0a1 using GTK 3.18 and gnome-shell, so I'm guessing that the version of GTK being used doesn't play nicely with gtk_drag_set_icon_surface as implemented in bug 1186661.

Vasilica, do you mind letting me know what version of Ubuntu or GTK+ you were using? Thanks!

Comment 3

[Andrew Comminos [:acomminos]]

I'm able to reproduce this issue on a compositing window manager using GTK 3.4- the issue appears to be caused by a drawing path to use X11 shaped windows if the cairo surface passed into gtk_drag_set_icon_surface contains an alpha channel. Unfortunately, the effect is suboptimal as the region clipped by the SHAPE extension is defined as any region with 50% alpha (as defined in gdk_cairo_region_create_from_surface).

The following commit in GTK+ fixes the issue by permitting ARGB windows in the presence of a compositing WM:

https://git.gnome.org/browse/gtk+/commit/?id=0296f1a43a803c0783781f9941a840256304641f

This change was introduced in GTK 3.9.12.

I'm thinking we should fallback to using GdkPixbufs for versions of GTK before 3.9.12, as we currently do in nsDragService.cpp without a compositing window manager. Letting GTK heuristically derive shape information from the cairo surface is incredibly poor anyway, as seen in the bug's screenshots.

> // Transparent drag icons need, like a lot of transparency-related things,
> // a compositing X window manager
> if (!gdk_screen_is_composited(screen))
>   return false;

Comment 4

[Andrew Comminos [:acomminos]]

Attachment: MozReview Request: Bug 1249604 - Don't use gtk_drag_set_icon_surface on GTK versions that use X SHAPE for dnd with cairo. r?karlt

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

Comment 5

[Andrew Comminos [:acomminos]]

Comment on attachment 8721577 [details]
MozReview Request: Bug 1249604 - Don't use gtk_drag_set_icon_surface on GTK versions that use X SHAPE for dnd with cairo. r?karlt

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

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 8721577 [details]
MozReview Request: Bug 1249604 - Don't use gtk_drag_set_icon_surface on GTK versions that use X SHAPE for dnd with cairo. r?karlt

https://reviewboard.mozilla.org/r/35725/#review32433

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed30a5133273

Comment 8

[Carsten Book [:Tomcat]]

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

Comment 9

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

Karl, Andrew, do you think  this and bug 1260777 are ok to leave till 47? Or do you want to request uplift here?

Comment 10

[Karl Tomlinson (:karlt)]

Comment on attachment 8721577 [details]
MozReview Request: Bug 1249604 - Don't use gtk_drag_set_icon_surface on GTK versions that use X SHAPE for dnd with cairo. r?karlt

Thanks, Liz.  I somehow missed that this wasn't on 46.
I think this explains that why bug 1260777 didn't look so bad when I tested.
It lists 46beta6, 47.0a2, and 48.0a1 as affected, but I think there must be variations in how bad things are.

Approval Request Comment
[Feature/regressing bug #]: bug 8636826
[User impact if declined]:
wispy images while dragging on old distros.
This patch doesn't make things look as good as on newer distros, but it is a noticeable improvement.
[Describe test coverage new/current, TreeHerder]: none.
[Risks and why]: This is low risk because this is a well used path, already used in other configurations.
[String/UUID change made/needed]: none.

Comment 11

[Karl Tomlinson (:karlt)]

(In reply to Karl Tomlinson (ni?:karlt) from comment #10)
> [Feature/regressing bug #]: bug 8636826

bug 1186661, in fact.

Comment 12

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

Comment on attachment 8721577 [details]
MozReview Request: Bug 1249604 - Don't use gtk_drag_set_icon_surface on GTK versions that use X SHAPE for dnd with cairo. r?karlt

More polish for gtk3, noticeable enough to warrant uplift to beta 11

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a6c0d2903cb6

Comment 14

[Andrew Comminos [:acomminos]]

Thanks Karl, I agree that this is unlikely to cause any regressions.