Closed Bug 1249604 Opened 4 years ago Closed 4 years ago
[Linux] Broken icon shadow during customising process
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
58 bytes, text/x-review-board-request
[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]:  Dragging an item from toolbar to Panel Menu displays an empty rectangle instead the icon shadow. See screenshot: http://i.imgur.com/oEl1B4Z.png  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?
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;
Review commit: https://reviewboard.mozilla.org/r/35725/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/35725/
Attachment #8721577 - Flags: review?(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 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+
Karl, Andrew, do you think this and bug 1260777 are ok to leave till 47? Or do you want to request uplift here?
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.
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.
You need to log in before you can comment on or make changes to this bug.