Closed Bug 1249604 Opened 4 years ago Closed 4 years ago

[Linux] Broken icon shadow during customising process

Categories

(Core :: Widget: Gtk, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox44 --- unaffected
firefox45 --- unaffected
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: vasilica.mihasca, Assigned: acomminos)

References

Details

(Keywords: regression)

Attachments

(1 file)

[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.
Looks like a regression from bug 1188138 or bug 1186661, yes. Andrew, can you take a look, please?
Blocks: 1188138, 1186661
Component: Toolbars and Customization → Widget: Gtk
Flags: needinfo?(andrew)
Product: Firefox → Core
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!
Flags: needinfo?(andrew) → needinfo?(vasilica.mihasca)
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;
Flags: needinfo?(vasilica.mihasca)
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/
Assignee: nobody → andrew
Status: NEW → ASSIGNED
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
Attachment #8721577 - Flags: review?(karlt) → review+
https://hg.mozilla.org/mozilla-central/rev/ed30a5133273
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Karl, Andrew, do you think  this and bug 1260777 are ok to leave till 47? Or do you want to request uplift here?
Flags: needinfo?(karlt)
Flags: needinfo?(andrew)
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.
Flags: needinfo?(karlt)
Attachment #8721577 - Flags: approval-mozilla-beta?
(In reply to Karl Tomlinson (ni?:karlt) from comment #10)
> [Feature/regressing bug #]: bug 8636826

bug 1186661, in fact.
No longer blocks: 1188138
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
Attachment #8721577 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Thanks Karl, I agree that this is unlikely to cause any regressions.
Flags: needinfo?(andrew)
You need to log in before you can comment on or make changes to this bug.