[GTK3] Tooltip text color wrong (elementary OS)

VERIFIED FIXED in Firefox 49

Status

()

defect
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: freaktechnik, Assigned: karlt)

Tracking

(Blocks 2 bugs, {regression})

Trunk
mozilla50
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 wontfix, firefox48 wontfix, firefox49+ fixed, firefox50 verified)

Details

(Whiteboard: [uplift to aurora after bug 1277818 and bug 1271523])

Attachments

(4 attachments)

Reporter

Description

3 years ago
Tooltips with the default theme on elementary OS 0.3 Freya have black instead of white text on a dark background. The background color (including its alpha channel) and tooltip shape seem correct.
Reporter

Updated

3 years ago
Blocks: gtk3
Assignee

Comment 1

3 years ago
Is this with GTK 3.14?
Blocks: ship-gtk3
Reporter

Comment 2

3 years ago
3.14.15-0ubuntu1~14.04~ricotz1, yep.
Assignee

Comment 3

3 years ago
I wonder whether the gtk_widget_set_name(widget, "MozillaGtkWidget") is
causing a failure to match theme css selectors

https://dxr.mozilla.org/mozilla-central/rev/d848a5628d801a460a7244cbcdea22d328d8b310/widget/gtk/gtk3drawing.c#513

https://bugzilla.mozilla.org/show_bug.cgi?id=1197165#c17
See Also: → 1197165
Assignee

Comment 4

3 years ago
Bug 1197165 made a change that is likely to help here.
Would you be able to test a recent Nightly, please?
Depends on: 1197165
Reporter

Comment 5

3 years ago
I've tested this in Nightly 20160308030418 and the tooltips are identical to the screenshots I attached earlier.
Reporter

Comment 6

3 years ago
Actually, sometimes they background isn't transparent anymore in this nightly, which is a bug.
Reporter

Comment 7

3 years ago
Sorry for the spam, but that isn't actually a new bug and was already present at the point I took the screenshots (tested it on the same build), it seems the background of a tooltip gets redrawn repeatedly without clearing. Is there already a bug for that or should I open a new one?
Assignee

Comment 8

3 years ago
I can reproduce the foreground color bug with GTK+ 3.16.7 and https://launchpad.net/egtk/4.x/4.0.6/+download/elementary.tar.xz

ISTR a bug report involving multiple tooltip text appearing/overlapping in the same place, but I don't have the number sorry.  Not sure whether that is what comment 6 and 7 described.  I'm not reproducing here.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee

Comment 9

3 years ago
Gecko gets the foreground color at
https://dxr.mozilla.org/mozilla-central/rev/05c087337043dd8e71cc27bdb5b9d55fd00aaa26/widget/gtk/nsLookAndFeel.cpp#1006

which doesn't match what the theme is expecting:

.tooltip GtkLabel,
.overlay-bar GtkLabel {
    color: white;
    text-shadow: 0 1px 2px alpha (#000, 0.6);
}
Assignee

Comment 10

3 years ago
(In reply to Martin Giger [:freaktechnik] from comment #7)
> it seems the background of a tooltip gets redrawn repeatedly without
> clearing. Is there already a bug for that or should I open a new one?

Bug 1258086 exists now.
Assignee

Updated

3 years ago
See Also: → 1268241
Assignee

Updated

3 years ago
Duplicate of this bug: 1276596
Assignee

Comment 12

3 years ago
GTK draws tooltip text in a GtkLabel in a GtkBox in the GtkWindow for the
tooltip.

3.20 Adwaita matches the style context using

  tooltip * {
    padding: 4px;
    background-color: transparent;
    color: white; }

I'd prefer not to keep additional style contexts for box and label children
cached because these will only be used once.  I suspect we'll need labels at
least for text colors of other widgets too, such as menuitem and menubar, and
so I think the best way to provide what nsLookAndFeel needs may be to provide
CreateGtkBoxStyle(GtkStyleContext* aParentStyle) and
CreateGtkLabelStyle(GtkStyleContext* aParentStyle) methods from
WidgetStyleCache.  These can use CreateCSSNode().
Or making CreateCSSNode() available would be another option.
Assignee

Updated

3 years ago
See Also: → 1271523
I think the best is to make CreateCSSNode available and allows it to create a new style from path without parent. I'll post such patch as a separate bug.
Depends on: 1277818
Assignee

Updated

3 years ago
Assignee: nobody → karlt
Status: NEW → ASSIGNED
Comment on attachment 8760611 [details]
Bug 1250704 - use style from WidgetStyleCache for tooltip colors.

https://reviewboard.mozilla.org/r/58146/#review55080
Attachment #8760611 - Flags: review?(stransky) → review+
Comment on attachment 8760612 [details]
Bug 1250704 - use same widget heirarchy as GTK for tooltip text color.

https://reviewboard.mozilla.org/r/58148/#review55326

Looks OK to me, tested on 3.20 and pre 3.20. The floating reference is a bit unusual to me here but it's documented in the header file.

There's one missing piece - the default Gtk style Adwaita defines padding for tooltip and also for tooltip * but we use the top-level only. The Firefox tooltips are considerably smaller then - but we can address that later.
Attachment #8760612 - Flags: review?(stransky) → review+
It needs Bug 1271523 land prior this one.
Depends on: 1271523
Keywords: checkin-needed

Comment 21

3 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6f125b1088b
use style from WidgetStyleCache for tooltip colors. r=stransky
https://hg.mozilla.org/integration/mozilla-inbound/rev/af36af25ecf4
use same widget heirarchy as GTK for tooltip text color. r=stransky
Keywords: checkin-needed
sorry had to back this out and the depend bug for leaks like https://treeherder.mozilla.org/logviewer.html#?job_id=29775389&repo=mozilla-inbound
Flags: needinfo?(karlt)
(In reply to Carsten Book [:Tomcat] from comment #22)
> sorry had to back this out and the depend bug for leaks like
> https://treeherder.mozilla.org/logviewer.html#?job_id=29775389&repo=mozilla-
> inbound

I wonder if these are really leaks, or if it is just we don't clean up the cache on shutdown.
Assignee

Comment 25

3 years ago
128 bytes in 4 blocks are definitely lost in loss record 7,790 of 9,085
   at 0x4C293D5: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0xB33DE08: g_malloc (gmem.c:159)
   by 0xB3529CF: g_slice_alloc (gslice.c:1003)
   by 0xB352EDD: g_slice_copy (gslice.c:1051)
   by 0xB0AFCDF: boxed_proxy_lcopy_value (gboxed.c:252)
   by 0x7C86D52: gtk_style_context_get_valist (gtkstylecontext.c:1534)
   by 0x7C86F32: gtk_style_context_get (gtkstylecontext.c:1569)
   by 0x7B04227: set_color (gtkstyle.c:651)
   by 0x7B0457D: gtk_style_update_from_context (gtkstyle.c:717)
   by 0x7B045D1: gtk_style_constructed (gtkstyle.c:782)
   by 0xB0BC08E: g_object_newv (gobject.c:1747)
   by 0xB0BC65C: g_object_new_valist (gobject.c:1836)
   by 0xB0BC970: g_object_new (gobject.c:1551)
   by 0x7B010E6: _gtk_style_new_for_path (gtkstyle.c:826)
   by 0x7B01145: gtk_style_new (gtkstyle.c:853)
   by 0x7B0118C: gtk_widget_get_default_style (gtkstyle.c:4005)
   by 0x7D34CC5: gtk_widget_init (gtkwidget.c:3662)
   by 0xB0D3810: g_type_create_instance (gtype.c:1882)
   by 0xB0BC9B8: g_object_constructor (gobject.c:1855)
   by 0xB0BBEB0: g_object_newv (gobject.c:1719)
   by 0xB0BC98B: g_object_new (gobject.c:1548)
   by 0x7D44030: gtk_window_new (gtkwindow.c:1624)
   by 0xEB73D27: CreateWindowWidget() (WidgetStyleCache.cpp:30)
   by 0xEB7693D: CreateTooltipWidget (WidgetStyleCache.cpp:113)
   by 0xEB7693D: CreateWidget (WidgetStyleCache.cpp:148)
   by 0xEB7693D: GetWidget(WidgetNodeType) (WidgetStyleCache.cpp:160)
   by 0xEB776C7: GetWidgetStyleInternal (WidgetStyleCache.cpp:318)
   by 0xEB776C7: ClaimStyleContext(WidgetNodeType, GtkTextDirection, GtkStateFlags, unsigned int) (WidgetStyleCache.cpp:353)
   by 0xEB8B9A8: nsLookAndFeel::Init() (nsLookAndFeel.cpp:1142)
   by 0xEB65973: nsXPLookAndFeel::GetInstance() (nsXPLookAndFeel.cpp:264)
   by 0xEB65E46: mozilla::LookAndFeel::GetInt(mozilla::LookAndFeel::IntID, int*) (nsXPLookAndFeel.cpp:912)
   by 0xD757BFC: GetInt (LookAndFeel.h:561)
   by 0xD757BFC: nsChromeRegistryChrome::CheckForOSAccessibility() (nsChromeRegistryChrome.cpp:160)
   by 0xF18F496: ScopedXPCOMStartup::SetWindowCreator(nsINativeAppSupport*) (nsAppRunner.cpp:1620)
   by 0xF193547: XREMain::XRE_mainRun() (nsAppRunner.cpp:4121)
   by 0xF194452: XREMain::XRE_main(int, char**, nsXREAppData const*) (nsAppRunner.cpp:4495)
   by 0xF1946D7: XRE_main (nsAppRunner.cpp:4603)
   by 0x405135: do_main(int, char**, char**, nsIFile*) (nsBrowserApp.cpp:254)
   by 0x40473D: main (nsBrowserApp.cpp:427)

Try testing confirms this is triggered by
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6f125b1088b
which changed the callstack of the first GtkWidget creation.

good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ff010a2005a
bad: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b11d7c6ccfa6

Modifying the existing suppression takes care of that:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ff2632fab0c
Flags: needinfo?(karlt)
Assignee

Comment 26

3 years ago
(In reply to Martin Stránský from comment #19)
> The floating reference is a bit
> unusual to me here but it's documented in the header file.

Yes, GtkWidget's initially unowned behavior seems weird to me, like
nsISupports, but more explicit.  The fact that GTK chose this so that parent
containers can assume ownership made me feel less bad about the change from
unowned to deleted here.

> There's one missing piece - the default Gtk style Adwaita defines padding
> for tooltip and also for tooltip * but we use the top-level only. The
> Firefox tooltips are considerably smaller then - but we can address that
> later.

Oh, that's interesting, thanks.  Looks like that would match both the box and
the label, apply the padding twice, in addition to the tooltip padding.

If we should be drawing box and/or label background, and it seems that GTK
does, then I guess we need to add TOOLTIP_BOX and/or TOOLTIP_LABEL
WidgetNodeTypes.

If it were just the border/padding, then that could be calculated once and
saved instead of the style context, but using WidgetStyleCache is probably
simpler.
Assignee

Comment 27

3 years ago
Review commit: https://reviewboard.mozilla.org/r/58652/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58652/
Attachment #8760611 - Attachment description: bug 1250704 use style from WidgetStyleCache for tooltip colors → Bug 1250704 - use style from WidgetStyleCache for tooltip colors.
Attachment #8760612 - Attachment description: bug 1250704 use same widget heirarchy as GTK for tooltip text color → Bug 1250704 - use same widget heirarchy as GTK for tooltip text color.
Attachment #8761488 - Flags: review?(n.nethercote)
Assignee

Comment 28

3 years ago
Comment on attachment 8760611 [details]
Bug 1250704 - use style from WidgetStyleCache for tooltip colors.

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

Comment 29

3 years ago
Comment on attachment 8760612 [details]
Bug 1250704 - use same widget heirarchy as GTK for tooltip text color.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58148/diff/1-2/
Comment on attachment 8761488 [details]
bug 1250704 relax suppresion of GtkStyle set_color leak to accept change in caller

https://reviewboard.mozilla.org/r/58652/#review55632
Attachment #8761488 - Flags: review?(n.nethercote) → review+

Comment 31

3 years ago
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/80425dc038ac
use style from WidgetStyleCache for tooltip colors. r=stransky
https://hg.mozilla.org/integration/mozilla-inbound/rev/daea8ab4c159
use same widget heirarchy as GTK for tooltip text color. r=stransky
https://hg.mozilla.org/integration/mozilla-inbound/rev/eebf02e5a399
relax suppresion of GtkStyle set_color leak to accept change in caller r=njn

Comment 32

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/80425dc038ac
https://hg.mozilla.org/mozilla-central/rev/daea8ab4c159
https://hg.mozilla.org/mozilla-central/rev/eebf02e5a399
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Assignee

Comment 33

3 years ago
[Tracking Requested - why for this release]:
This was a regression from the GTK3 port.  The issue has been reported for a number of themes.

Dependencies are required to land the patches here on 49.

Uplift to 48 may be possible also with possibly more dependencies.
Depends on: 1280951
Hi Karl, should we uplift this to Beta48 and Aurora49? This doesn't seem severe enough to be included in 47 dot release.
Flags: needinfo?(karlt)
Assignee

Comment 35

3 years ago
(In reply to Ritu Kothari (:ritu) from comment #34)
> Hi Karl, should we uplift this to Beta48 and Aurora49?

I think we should uplift to Aurora49, thanks.

Uplift to Beta48 would require uplift of bug 1234158 and bug 1271477, and I expect others including bug 1248974, bug 1272194, bug 1271579, and bug 1274745.
That may be worthwhile to fix 1248974, which has those dependencies also, but there is significant risk in all that.

How does that sound to you?
If you are not scared off, then I can start on beta uplift requests.
Flags: needinfo?(karlt) → needinfo?(rkothari)
Whiteboard: [uplift to aurora after bug 1234158 and bug 1271523]
(In reply to Karl Tomlinson (ni?:karlt) from comment #35)
> (In reply to Ritu Kothari (:ritu) from comment #34)
> > Hi Karl, should we uplift this to Beta48 and Aurora49?
> 
> I think we should uplift to Aurora49, thanks.
> 
> Uplift to Beta48 would require uplift of bug 1234158 and bug 1271477, and I
> expect others including bug 1248974, bug 1272194, bug 1271579, and bug
> 1274745.
> That may be worthwhile to fix 1248974, which has those dependencies also,
> but there is significant risk in all that.
> 
> How does that sound to you?
> If you are not scared off, then I can start on beta uplift requests.
NI Sylvestre and Gerry. I think we are getting close to mid-Beta48 and not sure if we want to uplift patches from 7 dependent bugs to fix this in 48. Perhaps it could wait for 49. Sylvestre owns 48 and will make a final decision.
Flags: needinfo?(sledru)
Flags: needinfo?(rkothari)
Flags: needinfo?(gchang)
Not interested for 48 as we already shipped 47 with it and we would like to avoid regressions.
thanks!
Flags: needinfo?(sledru)
Flags: needinfo?(gchang)
Assignee

Updated

3 years ago
Whiteboard: [uplift to aurora after bug 1234158 and bug 1271523] → [uplift to aurora after bug 1277818 and bug 1271523]
Assignee

Comment 38

3 years ago
Comment on attachment 8760611 [details]
Bug 1250704 - use style from WidgetStyleCache for tooltip colors.

Approval Request Comment

Please consider this a request for all patches for this bug.

[Feature/regressing bug #]: GTK3 port
[User impact if declined]:
incorrect tooltip text color, which is unreadable with some themes.
[Describe test coverage new/current, TreeHerder]:
manual testing only.
[Risks and why]: 
There is a chance of unexpected consequences, but this has been on m-c since 2016-06-10.
The only unexpected issue reported was a small increase in resource usage in a synthetic test that previously didn't use tooltips at all.  There is the option of uplifting bug 1280951 to address that, but in real usage that wasn't really a regression because previous code would have consumed those resources on first use of tooltips anyway.
[String/UUID change made/needed]:
none.
Attachment #8760611 - Flags: approval-mozilla-aurora?
Tracking 49+ for this visible tooltip issue.
Assignee

Updated

3 years ago
Blocks: 1268464
Comment on attachment 8760611 [details]
Bug 1250704 - use style from WidgetStyleCache for tooltip colors.

More GTK tooltip cleanup, ok to uplift to aurora.
Attachment #8760611 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Hello! I tried to reproduce and to verify this issue on Nightly (2016-03-08), respectively latest Nightly 50.0a1 (2016-06-29), using the conditions mentioned in comment 8, but https://launchpad.net/egtk/4.x/4.0.6/+download/elementary.tar.xz can't be installed. Firefox throw this error "This add-on cannot be installed because it appears to be corrupt". Can you provide a proper file?
Flags: needinfo?(martin)
Reporter

Comment 43

3 years ago
It is a GTK theme, not a Firefox theme. So it has to be installed to your system. Elementary OS ships with it by default for example, I don't know if there are other easy ways to use this GTK theme.

This issue is about colors that are given by the system not getting correctly applied by Firefox, which is why you need to use certain system themes.
Flags: needinfo?(martin)
(In reply to Martin Giger [:freaktechnik] from comment #43)
> It is a GTK theme, not a Firefox theme. So it has to be installed to your
> system. Elementary OS ships with it by default for example, I don't know if
> there are other easy ways to use this GTK theme.
> 
> This issue is about colors that are given by the system not getting
> correctly applied by Firefox, which is why you need to use certain system
> themes.

Thanks for your clarification! Unfortunately, we do not have available this Linux distribution. Considering these, can you please try to verify this bug?
Reporter

Comment 45

3 years ago
Yes, this is fixed for me in the latest nightly (2016-06-30), and has been for quite a while in nightly.
Status: RESOLVED → VERIFIED

Comment 46

3 years ago
(In reply to Martin Giger [:freaktechnik] from comment #45)
> Yes, this is fixed for me in the latest nightly (2016-06-30), and has been
> for quite a while in nightly.

Can you please clarify which FF version you were referring to? I ask because Iulia Cristescu is referring to firefox50 and I am using FF 48.0b10. So, your comment is unclear.
Reporter

Comment 47

3 years ago
(In reply to Ineuw from comment #46)
> Can you please clarify which FF version you were referring to? I ask because
> Iulia Cristescu is referring to firefox50 and I am using FF 48.0b10. So,
> your comment is unclear.

IT is not, as the date clearly identifies the version of Firefox I was using (which was version 50 at that point, and as the flags in the bug indicate the bug is fixed from 49 on and not earlier).

Comment 48

3 years ago
(In reply to Martin Giger [:freaktechnik] from comment #47)
> (In reply to Ineuw from comment #46)
> > Can you please clarify which FF version you were referring to? I ask because
> > Iulia Cristescu is referring to firefox50 and I am using FF 48.0b10. So,
> > your comment is unclear.
> 
> IT is not, as the date clearly identifies the version of Firefox I was using
> (which was version 50 at that point, and as the flags in the bug indicate
> the bug is fixed from 49 on and not earlier).

Thanks Martin, will patiently wait for the release.

Comment 49

3 years ago
Not fixed for me in 49. Tooltip background is corresponding to tooltip background in KDE application appearance settings, when tooltip text is corresponding to window text in settings (instead of tooltip text).
(In reply to etendren from comment #49)
> Not fixed for me in 49. Tooltip background is corresponding to tooltip
> background in KDE application appearance settings, when tooltip text is
> corresponding to window text in settings (instead of tooltip text).

Can you please clarify if this issue is running the Mozilla official released version, or the version provided by your Linux distributor?

Comment 51

3 years ago
(In reply to Bill Gianopoulos [:WG9s] from comment #50)
> (In reply to etendren from comment #49)
> > Not fixed for me in 49. Tooltip background is corresponding to tooltip
> > background in KDE application appearance settings, when tooltip text is
> > corresponding to window text in settings (instead of tooltip text).
> 
> Can you please clarify if this issue is running the Mozilla official
> released version, or the version provided by your Linux distributor?

official, downloaded from mozilla site.
(In reply to Vladimir from comment #51)
> (In reply to Bill Gianopoulos [:WG9s] from comment #50)
> > (In reply to etendren from comment #49)
> > > Not fixed for me in 49. Tooltip background is corresponding to tooltip
> > > background in KDE application appearance settings, when tooltip text is
> > > corresponding to window text in settings (instead of tooltip text).
> > 
> > Can you please clarify if this issue is running the Mozilla official
> > released version, or the version provided by your Linux distributor?
> 
> official, downloaded from mozilla site.

OK great! so it would seem this is a legitimate issue.  SO You should file a new bug about your KDE issue explaining what KDE theme you are using and showing screenshots illustrating what the issue is.  Although the subject of this bug seems very generic, it really was a bug about GNome 3 with the default adwaita theme and has been fixed.  We need a new bug on your KDE issue that blocks bug 1264079.

Comment 53

3 years ago
Using FF beta 50.0b2 on a 32bit Xubuntu 16.04 desktop, there is no change in the balloon display. It's still is black on black. On the other hand, my laptop with the identical but 64bit software, the balloons are white characters on a black background.
You need to log in before you can comment on or make changes to this bug.