Closed Bug 1365556 Opened 3 years ago Closed 3 years ago

Firefox 53 ButtonText incorrect with some themes and GTK 3.10

Categories

(Core :: Widget: Gtk, defect, P3)

53 Branch
x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- wontfix
firefox56 --- verified
firefox57 --- fixed

People

(Reporter: i.dani, Assigned: karlt)

References

Details

(Keywords: polish, regression, Whiteboard: tpi:+)

Attachments

(4 files)

Attached image brokenff53.png
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170419190606

Steps to reproduce:

Update from Firefox 52 to 53


Actual results:

Firefox overrides the default window manager font color in some of its pop up dialog and buttons (included buttons rendered in webpages)
This leads to unreadable buttons text.

See the top of the attached Screenshot1.png for an example: The "Quit" "Save" "Save and Quit" text is white (should be black). Hovering over the "Do no ask next time" also turns that text white.

Problem appeared after update of firefox:amd64 (from 52.0.2+build1-0ubuntu0.14.04.1 to 53.0+build6-0ubuntu0.14.04.1).

Using the default Firefox theme or the other 2 new "compact" provided themes produces the same result.

Confirmed on:
Ubuntu GNOME 14.04.5
Linux Mint 17.1 Rebecca - Cinnamon 2.4.8
On Xubuntu 14.04 with XFCE 4.10:
In the snapshot above the XFCE theme is the standard "Adwaita" but the problem can be reproduced with other themes thought it is not present for example with the similar XFCE theme "Orion".


Expected results:

Use the correct color, as it was in Firefox 52
Is it reproducible with the official Firefox build from Mozilla?
Component: Untriaged → Widget: Gtk
Flags: needinfo?(i.dani)
OS: Unspecified → Linux
Product: Firefox → Core
Hardware: Unspecified → x86_64
If have just tested with the latest official nightly build (v55)[1] and the problem is the same.

[1] https://www.mozilla.org/de/firefox/channel/desktop/
Flags: needinfo?(i.dani)
I've just tried some more stuff:
- Safe mode -> Same Problem
- Refresh FF (https://support.mozilla.org/en-US/kb/refresh-firefox-reset-add-ons-and-settings) -> Same Problem
- Purge FF, clean up, Fresh install -> Same Problem

Here is what "about:support" says:


Application Basics
------------------

Name: Firefox
Version: 53.0.2
Build ID: 20170509210304
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:53.0)
Gecko/20100101 Firefox/53.0
OS: Linux 3.19.8-ckt22-openslx
Multiprocess Windows: 0/1 (Disabled by add-ons)
Safe Mode: false

Crash Reports for the Last 3 Days
---------------------------------

All Crash Reports

Extensions
----------

Name: Application Update Service Helper
Version: 2.0
Enabled: true
ID: aushelper@mozilla.org

Name: Multi-process staged rollout
Version: 1.14
Enabled: true
ID: e10srollout@mozilla.org

Name: Pocket
Version: 1.0.5
Enabled: true
ID: firefox@getpocket.com

Name: Ubuntu Modifications
Version: 3.2
Enabled: true
ID: ubufox@ubuntu.com

Name: Web Compat
Version: 1.0
Enabled: true
ID: webcompat@mozilla.org

Graphics
--------

Features
Compositing: Basic
Asynchronous Pan/Zoom: none
WebGL Renderer: Intel Open Source Technology Center -- Mesa DRI Intel(R)
Haswell Desktop
WebGL2 Renderer: WebGL creation failed: * WebGL 2 requires support for
the following features: transform_feedback2 * Exhausted GL driver options.
Audio Backend: pulse
GPU #1
Active: Yes
Description: Intel Open Source Technology Center -- Mesa DRI Intel(R)
Haswell Desktop
Vendor ID: Intel Open Source Technology Center
Device ID: Mesa DRI Intel(R) Haswell Desktop
Driver Version: 3.0 Mesa 10.1.3

Diagnostics
AzureCanvasAccelerated: 0
AzureCanvasBackend: skia
AzureContentBackend: skia
AzureFallbackCanvasBackend: none
CairoUseXRender: 0
Decision Log
HW_COMPOSITING:
blocked by default: Acceleration blocked by platform
OPENGL_COMPOSITING:
unavailable by default: Hardware compositing is disabled




Important Modified Preferences
------------------------------

browser.cache.disk.capacity: 1048576
browser.cache.disk.filesystem_reported: 1
browser.cache.disk.smart_size.first_run: false
browser.cache.frecency_experiment: 1
browser.download.importedFromSqlite: true
browser.places.smartBookmarksVersion: 8
browser.sessionstore.upgradeBackup.latestBuildID: 20170509210304
browser.startup.homepage_override.buildID: 20170509210304
browser.startup.homepage_override.mstone: 53.0.2
browser.tabs.remote.autostart.2: true
browser.urlbar.daysBeforeHidingSuggestionsPrompt: 3
browser.urlbar.lastSuggestionsPromptDate: 20170522
extensions.lastAppVersion: 53.0.2
media.gmp.storage.version.observed: 1
network.cookie.prefsMigrated: true
places.history.expiration.transient_current_max_pages: 60313
plugin.disable_full_page_plugin_for_types: application/pdf

Important Locked Preferences
----------------------------

Places Database
---------------

JavaScript
----------

Incremental GC: true

Accessibility
-------------

Activated: false
Prevent Accessibility: 0

Library Versions
----------------

NSPR
Expected minimum version: 4.13.1
Version in use: 4.13.1

NSS
Expected minimum version: 3.29.5
Version in use: 3.29.5

NSSSMIME
Expected minimum version: 3.29.5
Version in use: 3.29.5

NSSSSL
Expected minimum version: 3.29.5
Version in use: 3.29.5

NSSUTIL
Expected minimum version: 3.29.5
Version in use: 3.29.5

Experimental Features
---------------------

Sandbox
-------

Seccomp-BPF (System Call Filtering): true
Seccomp Thread Synchronization: true
User Namespaces: true
Media Plugin Sandboxing: true
Have you tried the workaround I posted at https://bugzilla.mozilla.org/show_bug.cgi?id=1358091#c3 ?

Also, for UI buttons you can create userChrome.css in that same chrome folder with the following:

.button-text,
.button-text:hover,
.button-text:active:hover,
.button-text:-moz-focusring::-moz-focus-inner,
toolbarbutton,
toolbarbutton:hover,
toolbarbutton:active:hover {
  color: black !important;
}


I don't think it'll get all the instances of the problem but surely most of them.
Still, it being a workaround, I'd much prefer an actual bugfix.
Keywords: polish
Priority: -- → P3
Whiteboard: tpi:+
(In reply to velenir from comment #6)
> Have you tried the workaround I posted at
> https://bugzilla.mozilla.org/show_bug.cgi?id=1358091#c3 ?
> 
> Also, for UI buttons you can create userChrome.css in that same chrome
> folder with the following:
> 
> .button-text,
> .button-text:hover,
> .button-text:active:hover,
> .button-text:-moz-focusring::-moz-focus-inner,
> toolbarbutton,
> toolbarbutton:hover,
> toolbarbutton:active:hover {
>   color: black !important;
> }
> 
> 
> I don't think it'll get all the instances of the problem but surely most of
> them.
> Still, it being a workaround, I'd much prefer an actual bugfix.

I've tried this - sadly there are still quite a lot of missing parts.
So we really do not want to use this in our production environment.. 

Is there a fix comming anytime soon? It's quite annoying not beeing able to update because of this bug.
(In reply to i.dani from comment #0)
> Confirmed on:
> Ubuntu GNOME 14.04.5
> Linux Mint 17.1 Rebecca - Cinnamon 2.4.8
> On Xubuntu 14.04 with XFCE 4.10:
> In the snapshot above the XFCE theme is the standard "Adwaita" but the
> problem can be reproduced with other themes thought it is not present for
> example with the similar XFCE theme "Orion".

You can repro this on a standard Ubuntu install with Unity? I'm testing in that environment and I'm not seeing the issue. So I wonder what's different.
Flags: needinfo?(i.dani)
Blocks: 1369453
Allright i've setup multiple fresh and clean instances of Ubuntu.
I did the following step's on all of them:

1) Download and clean install
2) Install all updates and reboot
3) Run firefox

-> No extra packages or anything. Just a clean Ubuntu with the newest version of Firefox (v53.0.3)

- Ubuntu (Default - Unity) -- 14.04.05 LTS -- http://releases.ubuntu.com/14.04/ubuntu-14.04.5-desktop-amd64.iso
-> The font color is correct! No Problem exists.

- Ubuntu GNOME -- 14.04.05 LTS -- http://cdimage.ubuntu.com/ubuntu-gnome/releases/trusty/release/ubuntu-gnome-14.04.5-desktop-amd64.iso
-> The font color is wrong! Problem exists.

- Xubuntu (Xfce) with Greybird Theme (Default) -- 14.04.05 LTS -- http://cdimage.ubuntu.com/xubuntu/releases/14.04.2/release/xubuntu-14.04.5-desktop-amd64.iso
-> The font color is correct! No Problem exists.

- Xubuntu (Xfce) with  Adwaita Theme -- 14.04.05 LTS -- http://cdimage.ubuntu.com/xubuntu/releases/14.04.2/release/xubuntu-14.04.5-desktop-amd64.iso
-> The font color is wrong! Problem exists.

- Ubuntu GNOME -- 16.04.02 LTS -- http://cdimage.ubuntu.com/ubuntu-gnome/releases/16.04/release/ubuntu-gnome-16.04.2-desktop-amd64.iso
-> The font color is correct! No Problem exists.


----

Do you need anything more?
Flags: needinfo?(i.dani) → needinfo?(jmathies)
Karl, any suggestions? How do we handle minor theme conflict issues under Linux?
Flags: needinfo?(jmathies) → needinfo?(karlt)
Duplicate of this bug: 1358091
Duplicate of this bug: 1360821
https://bugzilla.mozilla.org/show_bug.cgi?id=1358091#c1 identifies ButtonText, thanks.

Adwaita is GTK's default theme (though not Ubuntu's override), and Ubuntu 14.04 is a LTS release, so this probably affects a moderate proportion of users.

The bug appears to be in reading the incorrect colour from the GTK theme, rather than a conflict with Firefox styling.

A regression window would provide the first clues.
I'm assuming this reproduces on any Ubuntu 14.04 install after switching to the Adwaita GTK theme.
Flags: needinfo?(karlt)
Summary: Firefox 53 conflicts with window manager theme → Firefox 53 ButtonText incorrect with some themes and GTK 3.10
Over to Tracy for regression range work.
Flags: needinfo?(twalker)
Too late for 54 but we could still take a fix in 55.
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #15)
> Too late for 54 but we could still take a fix in 55.

Can we expect a fix for FF55? This is affecting multiple users and it gets more and more critical when using an older version. Especially as there are multiple security vulnerabilities fixed in the latest version.. Thanks for taking a look at it!
Flags: needinfo?(lhenry)
Tracy, any luck with the regression range? Do you want to pass it over to SV?
Flags: needinfo?(lhenry)
Sorry for delay here.  This works for me on Ubuntu 16.04.02. I do not have an affected machine (VM). 

Which of the distros in comment #9 do we support?
Flags: needinfo?(twalker)
I can reproduce this using the build from comment 9, Ubuntu GNOME.

The regression range is:
Last good revision: 741a720c98cdb92c229376be0badbf036f653bff
First bad revision: 8f1e420699832d42753c6503aa31861be6f5b186
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=741a720c98cdb92c229376be0badbf036f653bff&tochange=8f1e420699832d42753c6503aa31861be6f5b186

Many bugs were fixed in this range, but it seems to be caused by Bug 1320860.

Screen capture if available here: https://testing-1.tinytake.com/sf/MTczNTc2NV81NzM0MzAz

needinfo me if additional testing is required.
I've just tested with the latest Firefox v55b7 on Ubuntu 14 with Gnome.
Some stuff is fixed, some stuff is still broken (sadly :-/)

- Closing Firefox with Tab's open:
https://image.prntscr.com/image/lvtWG3HbTLaRYY8po0eJNw.png

- Firefox asking to set default browser:
https://image.prntscr.com/image/s2KlAHM_S5qTxlujDFX_aw.png

- "x" to close a tab:
https://image.prntscr.com/image/GJv4ASWpScCtIEaKNkqsIw.png

- Menü buttons are fixed! :-)
https://image.prntscr.com/image/CXwN0WulTrOCJSe_0MVVgw.png

Can we expect this to be fixed in the stable release?
Flags: needinfo?(twalker)
Flags: needinfo?(lhenry)
Flags: needinfo?(karlt)
Flags: needinfo?(jmathies)
- about:support info with Firefox v55b7:
https://pastebin.com/tWt9ftnt
(In reply to Abe - QA (:Abe_LV) from comment #19)
> I can reproduce this using the build from comment 9, Ubuntu GNOME.
> 
> The regression range is:
> Last good revision: 741a720c98cdb92c229376be0badbf036f653bff
> First bad revision: 8f1e420699832d42753c6503aa31861be6f5b186
> Pushlog:
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=741a720c98cdb92c229376be0badbf036f653bff&tochange=8f1e
> 420699832d42753c6503aa31861be6f5b186
> 
> Many bugs were fixed in this range, but it seems to be caused by Bug 1320860.
> 
> Screen capture if available here:
> https://testing-1.tinytake.com/sf/MTczNTc2NV81NzM0MzAz
> 
> needinfo me if additional testing is required.

Thank you for looking into this.

https://hg.mozilla.org/mozilla-central/rev/a6aabcb07a6e from that range for
bug 1320860 links to
https://archive.mozilla.org/pub/firefox/nightly/2016/12/2016-12-01-03-02-05-mozilla-central/
for "first release with".

However, I can't reproduce the bad foreground button text color with
data:text/html,<button>Button</button> nor the bad hover color on "You are
about to close 3 tabs. Are you sure you want to continue?", after attempting
to closing the window.  (I don't know how to get the "Quit" / "Save and Quit"
dialog.)

Also, a6aabcb07a6e causes other regressions and was backed out before
relanding in a different form.  I think the transparent menu background was a
different issue.

Are you able to find a regression range or ranges for foreground button text
color with data:text/html,<button>Button</button> and bad hover text color on
"You are about to close 3 tabs. Are you sure you want to continue?", please?
Flags: needinfo?(amasresha)
(In reply to i.dani from comment #20)
> Can we expect this to be fixed in the stable release?

I don't know.  The button text color was still not correct when I tested with a 2017-07-01 Nightly.

I'm also seeing many scary GObject warnings with GTK 3.10 (even with other themes).  Are you getting these?:

(firefox:11284): GLib-GObject-CRITICAL **: g_object_ref: assertion 'object->ref_count > 0' failed

(firefox:11284): GLib-GObject-CRITICAL **: g_object_unref: assertion 'object->ref_count > 0' failed
Flags: needinfo?(karlt) → needinfo?(i.dani)
> > The regression range is:
> > Last good revision: 741a720c98cdb92c229376be0badbf036f653bff
> > First bad revision: 8f1e420699832d42753c6503aa31861be6f5b186
> > Pushlog:
> > https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=741a720c98cdb92c229376be0badbf036f653bff&tochange=8f1e420699832d42753c6503aa31861be6f5b186

> I think the transparent menu background was a different issue.

Yes, just noticed. Do we have a separate bug for this? If no, we may need to file one.
 
> Are you able to find a regression range or ranges for foreground button text
> color with data:text/html,<button>Button</button> and bad hover text color on
> "You are about to close 3 tabs. Are you sure you want to continue?", please?

The regression range for this(foreground button text color) is: 
Last good revision: ba62b4bebebfc30b3863d9b1fe220cff9c42f6f1
First bad revision: 8b922bd2f864d6c850f5624f26775348becd2365
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ba62b4bebebfc30b3863d9b1fe220cff9c42f6f1&tochange=8b922bd2f864d6c850f5624f26775348becd2365

It seems to be caused by Bug 1320686
Blocks: 1320686
Flags: needinfo?(amasresha)
Martin, if you have a chance, can you take a look at this, since you worked on bug 1320686?
Flags: needinfo?(lhenry) → needinfo?(stransky)
Sure, I'll look at it.
Flags: needinfo?(stransky)
The problem is with getting the sButtonText color from label style created by CreateStyleForWidget(). Looks like the obtained style does not have correct parent and is matched as standalone label style instead of button label style. 

A quick workaround can be to use the old code on gtk 3.10:

style = gtk_widget_get_style_context(label);
gtk_style_context_get_color(style, GTK_STATE_FLAG_NORMAL, &color);
sButtonText = GDK_RGBA_TO_NS_RGBA(color);
gtk_style_context_get_color(style, GTK_STATE_FLAG_PRELIGHT, &color);
sButtonHoverText = GDK_RGBA_TO_NS_RGBA(color);
(In reply to Karl Tomlinson (:karlt) from comment #23)
> (In reply to i.dani from comment #20)
> > Can we expect this to be fixed in the stable release?
> 
> I don't know.  The button text color was still not correct when I tested
> with a 2017-07-01 Nightly.
> 
> I'm also seeing many scary GObject warnings with GTK 3.10 (even with other
> themes).  Are you getting these?:
> 
> (firefox:11284): GLib-GObject-CRITICAL **: g_object_ref: assertion
> 'object->ref_count > 0' failed
> 
> (firefox:11284): GLib-GObject-CRITICAL **: g_object_unref: assertion
> 'object->ref_count > 0' failed

That doesn't seem to be related to this bug at least from my recent debugging - unfortunately gdb crashes for me on Ubuntu 14.04 so I need to solve that first.
Status: UNCONFIRMED → NEW
Ever confirmed: true
It's because GetWidget(MOZ_GTK_BUTTON) does not return a plain GtkButton (created by gtk_button_new()) as it was in original code but creates button by gtk_button_new_with_label("M").

Then we add another label style to it:
GtkStyleContext* labelStyle = CreateStyleForWidget(gtk_label_new("M"), style);

which causes the failure here.
Comment on attachment 8890480 [details]
Bug 1365556 - remove text-button class from MOZ_GTK_BUTTON style,

https://reviewboard.mozilla.org/r/161598/#review167246

::: commit-message-388d8:3
(Diff revision 1)
> +When GtkButton is created by gtk_button_new_with_label() it has text-button class.
> +When any child widget is added this class is removed. We want to emulate the GtkLabel
> +placed inside GtkButton so remove temporary text-button class on Gtk+ <= 3.10
> +where it causes issues with Adwaita theme.

Thank you for investigating this.

gtk_button_new_with_label() is the standard way to create a GtkLabel inside a GtkButton, and so the
text-button class that comes with that may be what we want.  Getting the style
from the child GtkLabel from gtk_button_new_with_label() may even be a
sensible approach.

I suspect the text-button class is not really the source of the problem here,
but the patch is changing invalidation or something that we don't understand.

In the Adwaita package from
https://packages.ubuntu.com/trusty/gnome-themes-standard-data
these are the only occurrences of text-button that I see:

.toolbar .button.text-button {
    padding: 2px 16px;
}

.header-bar .button.text-button {
    padding: 2px 16px;
}

Those shouldn't be involved here, and shouldn't affect the text color even if they were involved.
Attachment #8890480 - Flags: review?(karlt)
(In reply to Karl Tomlinson (:karlt) from comment #31)
> Comment on attachment 8890480 [details]
> Bug 1365556 - remove text-button class from MOZ_GTK_BUTTON style,
> 
> https://reviewboard.mozilla.org/r/161598/#review167246
> 
> ::: commit-message-388d8:3
> (Diff revision 1)
> > +When GtkButton is created by gtk_button_new_with_label() it has text-button class.
> > +When any child widget is added this class is removed. We want to emulate the GtkLabel
> > +placed inside GtkButton so remove temporary text-button class on Gtk+ <= 3.10
> > +where it causes issues with Adwaita theme.
> 
> Thank you for investigating this.
> 
> gtk_button_new_with_label() is the standard way to create a GtkLabel inside
> a GtkButton, and so the
> text-button class that comes with that may be what we want.  Getting the
> style
> from the child GtkLabel from gtk_button_new_with_label() may even be a
> sensible approach.
> 
> I suspect the text-button class is not really the source of the problem here,
> but the patch is changing invalidation or something that we don't understand.
> 
> In the Adwaita package from
> https://packages.ubuntu.com/trusty/gnome-themes-standard-data
> these are the only occurrences of text-button that I see:
> 
> .toolbar .button.text-button {
>     padding: 2px 16px;
> }
> 
> .header-bar .button.text-button {
>     padding: 2px 16px;
> }
> 
> Those shouldn't be involved here, and shouldn't affect the text color even
> if they were involved.

Yes, you're right. The .text-button class is not significant here, I think any subclass breaks that. The subclass was actually introduced at Gtk+ 3.10 and indicates a special case of GtkButton (text or image) and is not used much at 3.10.

I did Adwaita/Ambiance theme check and also Adwaita 3.10 vs. 3.14 theme check and it looks like a theme bug to me.

Ambiance 3.10 and Adwaita 3.14 both defines css .button color which works with .button and also .text-button subclass. Adwaita 3.10 does not define the color explicitly for .buttons which is the difference here and rely to inheritance here which is broken by .text-button subclass. 

I expect there's a bug in Gtk 3.10 which does not expect any sub-class at .button and that also break the color inheritance from upper class, maybe some css match optimization or so.

Anyway, I don't think it's worth to deep investigate this. A simple fix would be to add color to .button which we can't expect from Gtk team. The .text-button is also a freshly new at 3.10 and not widely used by Gtk - so looks like to me a good workaround to just remove it from button class for 3.10 as it does this patch.
Flags: needinfo?(karlt)
(In reply to Abe - QA (:Abe_LV) from comment #24)
> > > The regression range is:
> > > Last good revision: 741a720c98cdb92c229376be0badbf036f653bff
> > > First bad revision: 8f1e420699832d42753c6503aa31861be6f5b186
> > > Pushlog:
> > > https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=741a720c98cdb92c229376be0badbf036f653bff&tochange=8f1e420699832d42753c6503aa31861be6f5b186
> 
> > I think the transparent menu background was a different issue.
> 
> Yes, just noticed. Do we have a separate bug for this? If no, we may need to
> file one.
>  

Transparent menu background it a know bug of Adwaita 3.10 and Fedora 20 carries extra patch for it:
http://pkgs.fedoraproject.org/cgit/rpms/gnome-themes-standard.git/commit/?h=f20&id=3dcdc3cb629936821c39d52f5fff227aeeb014c2
The Button Boxes demo in gtk3-demo uses gtk_button_new_with_label(), which
presumably has button-text, and that renders correctly.  This is not a theme
bug.  The Adwaita rule that should provide the button color is

.background {
    color: @theme_fg_color;
    background-color: @theme_bg_color;
}

This background class is on the GtkWindow, and, as Martin implies, the color
property should be inherited by the descendant button and label, but this is
not happening.

Having the ability to reproduce a bug is an opportunity to find the cause.
Covering it would leave us not knowing when it will appear again.

'The main task of the engineering analyst is not merely to obtain “solu- 
tions” but is rather to understand the dynamic behavior of the system in 
such a way that the secrets of the mechanism are revealed, and that if it is 
built it will have no surprises left for him. Other than exhaustive physical 
experimentation, this is the only sound basis for engineering design, and 
disregard of this cardinal principle has not infrequently led to disaster.'
https://archive.org/details/AnalysisOfNonlinearControlSystemsByGrahamAndMcRuer

I'm not interested in adding code when we don't understand how it works around
a bug.  Such workarounds could just as easily introduce new regressions.
There are too many themes to test them all and use code just because it seems
to happen to make them work.

It is the invalidation here that causes the style to resolve correctly here,
not the removal of the button-text class.  Understanding exactly why and when
this invalidation is required permits us to resolve a whole class of bug,
rather than reproducing and trying patches for each bug in turn as they are
reported.  The understanding also gives us a chance to avoid making the same
mistakes in the future.
Flags: needinfo?(twalker)
Flags: needinfo?(karlt)
Flags: needinfo?(jmathies)
Attached file debug session log
The StyleData for the GtkButton, used in the incorrect style resolution here
was constructed when gtk_button_construct_child() retrieved the style property
"image-spacing", during construction of the GtkButton.

When the parent of the button is set to make the button (and label) a
descendant of the GtkWindow, _gtk_style_context_queue_invalidate() is called,
but the queued event is not processed before the color is retrieved for the
label.

This is essentially the same issue as observed in
https://bugzilla.mozilla.org/show_bug.cgi?id=1272194#c7
It affects GTK versions prior to 3.18.  More recent versions do not use
_gtk_style_context_queue_invalidate().

Queuing invalidations usually provides the expected behavior for GtkWidgets
because they usually retrieve style properties during measuring and drawing,
which are queued in a way that happens after the queued invalidation.
Gecko however is retrieving the style properties immediately and caching them.
They are never corrected after the invalidation.
Comment on attachment 8892371 [details]
bug 1365556 invalidate widget style contexts after their ancestors are set

https://reviewboard.mozilla.org/r/163326/#review168798

You rule.
Attachment #8892371 - Flags: review?(stransky) → review+
Thanks a lot for looking at this. Do you know in which release this will be published?
Flags: needinfo?(i.dani)
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/96777f4f483e
invalidate widget style contexts after their ancestors are set r=stransky+263117
https://hg.mozilla.org/mozilla-central/rev/96777f4f483e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Please request Beta approval on this when you get a chance.
Assignee: nobody → karlt
Flags: needinfo?(karlt)
Comment on attachment 8892371 [details]
bug 1365556 invalidate widget style contexts after their ancestors are set

Approval Request Comment
[Feature/Bug causing the regression]:
The bug has existed since bug 1193807 but was unnoticed or unreported until symptoms reported here were triggered by changes for bug 1320686.
[User impact if declined]:
Hard to read text on buttons in some themes, include GNOME's default theme in older LTS distributions.
Potentially some other natively themed widgets are also affects in some theme combinations.
[Is this code covered by automated tests?]:
No.
[Has the fix been verified in Nightly?]:
No.
[Needs manual test from QE? If yes, steps to reproduce]:
Yes.
Load data:text/html,<button>Button</button> on Ubuntu GNOME -- 14.04.05 LTS
[List of other uplifts needed for the feature/fix]:
None.
[Is the change risky?]:
Mildly risky.
[Why is the change risky/not risky?]:
There are too many different themes to test them all, and each GTK version behaves a little differently.  The patch should only correct bugs, but sometimes correcting one bug makes others visible.
[String changes made/needed]:
None.
Flags: needinfo?(karlt)
Attachment #8892371 - Flags: approval-mozilla-beta?
(In reply to Karl Tomlinson (:karlt) from comment #43)
> Comment on attachment 8892371 [details]
> bug 1365556 invalidate widget style contexts after their ancestors are set
> 
> Approval Request Comment
> [Feature/Bug causing the regression]:
> The bug has existed since bug 1193807 but was unnoticed or unreported until
> symptoms reported here were triggered by changes for bug 1320686.
> [User impact if declined]:
> Hard to read text on buttons in some themes, include GNOME's default theme
> in older LTS distributions.
> Potentially some other natively themed widgets are also affects in some
> theme combinations.
> [Is this code covered by automated tests?]:
> No.
> [Has the fix been verified in Nightly?]:
> No.
> [Needs manual test from QE? If yes, steps to reproduce]:
> Yes.
> Load data:text/html,<button>Button</button> on Ubuntu GNOME -- 14.04.05 LTS
> [List of other uplifts needed for the feature/fix]:
> None.
> [Is the change risky?]:
> Mildly risky.
> [Why is the change risky/not risky?]:
> There are too many different themes to test them all, and each GTK version
> behaves a little differently.  The patch should only correct bugs, but
> sometimes correcting one bug makes others visible.
> [String changes made/needed]:
> None.

I've just tested with the latest nightly release and for me everything is perfectly fine now. All wrong colors/buttons are now fixed and i couldn't find any other bug/wrong color. Using Default Install of Ubuntu 14.04.5 LTS.
Thanks for your work!
Thanks for testing.
Status: RESOLVED → VERIFIED
https://hg.mozilla.org/projects/date/rev/96777f4f483e77572640f671bf64a24f734488f9
bug 1365556 invalidate widget style contexts after their ancestors are set r=stransky+263117
Depends on: 1388810
Comment on attachment 8892371 [details]
bug 1365556 invalidate widget style contexts after their ancestors are set

Button/theme fix for GTK hard to read text. Let's fix this for accessibility in beta 2.
Attachment #8892371 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
No longer blocks: 1389133
Depends on: 1389133
Flags: qe-verify+
Reproduced the initial issue using Gnome on Ubuntu 14.04 32bit using Firefox 53.0 RC. Verified that the issue is no longer reproducible using Firefox 56 beta 11 on the same platform.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.