Bug 527729 - Apply Themefixes unless flag specified.

After further investigation, the style fixes seemed to be needed
for Ubutu's dark theme to function properly.

Solution:
- (as before) Apply Functional fixes (like keyboard navigation) at all
times.
- Apply theme fixes for all themes, but permit them to be
  turned off if the following Java vm arg is specified:
   -Dorg.eclipse.swt.internal.gtk.noThemingFixes
- (as before): Allow custom css to be loaded via:
   org.eclipse.swt.internal.gtk.cssFile.

This should fix Ubuntu's dark theme issue but still allow clients
to turn off special theme fixes.

Please reply with +1/-1 if you are in favor or against this approach.
This patch is fairly trivial, if you have an alternative suggestion,
please feel free to share.

Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=527729
Change-Id: Ib2e419ee29385a9261a3bef754ef92b168d161b4
Signed-off-by: Leo Ufimtsev <lufimtse@redhat.com>

7 files changed, 35 insertions, 21 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
index 48160eb14c..7b7685e406 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java
@@ -4230,6 +4230,8 @@ public static final native int strcmp (long /*int*/ s1, byte [] s2);
  * 		Adwaita
  * 		Adwaita-Dark
  * 		Ambiance 		(Ubuntu).
+ *
+ * See also: Device.overrideThemeValues();
  */
 public static final String getThemeName() {
 	byte[] themeNameBytes = getThemeNameBytes();
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_adwaita_gtk_pre320.css b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_adwaita_gtk_pre320.css
deleted file mode 100644
index 84f46e3ef9..0000000000
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_adwaita_gtk_pre320.css
+++ /dev/null
@@ -1,12 +0,0 @@
-GtkToolbar {
-	padding-top: 2px;
-	padding-bottom: 2px;
-}
-
-GtkToolbar GtkButton {
-	padding: 2px 4px 2px 4px;
-}
-
-GtkToolbar GtkMenuButton {
-	padding: 1px 0px 1px 0px;
-}
\ No newline at end of file
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_common_gtk_320.css b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_functional_gtk_320.css
index a74e9fbe4a..a74e9fbe4a 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_common_gtk_320.css
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_functional_gtk_320.css
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_common_gtk_pre320.css b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_functional_gtk_pre320.css
index 6a03f5c66f..4ed146c449 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_common_gtk_pre320.css
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_functional_gtk_pre320.css
@@ -1,3 +1,4 @@
+/* These CSS fixes have 'functinal' impact, no visual impact. E.g allow keyboard navigation in trees etc.. */
 .undershoot.top, .undershoot.right, .undershoot.bottom, .undershoot.lef	{
 	background-image: none;
 }
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_adwaita_gtk_320.css b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_theming_fixes_gtk_320.css
index fb0cccb789..fb0cccb789 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_adwaita_gtk_320.css
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_theming_fixes_gtk_320.css
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_theming_fixes_gtk_pre320.css b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_theming_fixes_gtk_pre320.css
new file mode 100644
index 0000000000..4b68e8a08b
--- /dev/null
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/swt_theming_fixes_gtk_pre320.css
@@ -0,0 +1,15 @@
+/* These have an impact on visual aspects of SWT. Padding etc.. 
+   There should not be hard-coded color fixes because this can break dark variants of themes.
+   Instead either use OS.setDarkThemePreferred() if your theme has dark version or style manually via api */
+GtkToolbar {
+	padding-top: 2px;
+	padding-bottom: 2px;
+}
+
+GtkToolbar GtkButton {
+	padding: 2px 4px 2px 4px;
+}
+
+GtkToolbar GtkMenuButton {
+	padding: 1px 0px 1px 0px;
+}
\ No newline at end of file
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Device.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Device.java
index 0bf581fc75..a0bdf6c886 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Device.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Device.java
@@ -720,27 +720,35 @@ private void overrideThemeValues () {
 
 	StringBuilder combinedCSS = new StringBuilder();
 
-	// Load CSS that is common to all themes. Gtk Css allows 'functional'  changes, such as keyboard shortcuts.
+	// Load functional CSS fixes. Such as keyboard functionality for some widgets.
 	combinedCSS.append(load.apply(
 		GTK.GTK_VERSION < OS.VERSION(3, 20, 0) ?
-				"/org/eclipse/swt/internal/gtk/swt_common_gtk_pre320.css" :
-				"/org/eclipse/swt/internal/gtk/swt_common_gtk_320.css"
+				"/org/eclipse/swt/internal/gtk/swt_functional_gtk_pre320.css" :
+				"/org/eclipse/swt/internal/gtk/swt_functional_gtk_320.css"
 			, true));
 
-	// Load CSS specific to Adwaita, to overcome things such as excessive padding that breaks SWT otherwise.
-	String OsThemeName = OS.getThemeName();
-	if ("Adwaita".equals(OsThemeName) || "Adwaita-dark".equals(OsThemeName)){
+	// By default, load CSS theme fixes to overcome things such as excessive padding that breaks SWT otherwise.
+	// Initially designed for Adwaita light/dark theme, but after investigation other themes (like Ubuntu's Ambiance + dark) seem to benefit from this also.
+	// However, a few themes break with these fixes, so we allow them to be turned off by user and allow them to load their own fixes manually instead.
+	// To turn on this flag, add the following vm argument:  -Dorg.eclipse.swt.internal.gtk.noThemingFixes
+	// Note:
+	// - Display.create() may override the theme name. See Display.create() ... OS.getThemeName(..).
+	// - These fixes should not contain any color information, otherwise it might break a light/dark variant of the theme.
+	//   Color fixes should be put either into the theme itself or via swt user api.
+	if (System.getProperty("org.eclipse.swt.internal.gtk.noThemingFixes") == null) {
 		combinedCSS.append(load.apply(
 				GTK.GTK_VERSION < OS.VERSION(3, 20, 0) ?
-						"/org/eclipse/swt/internal/gtk/swt_adwaita_gtk_pre320.css" :
-						"/org/eclipse/swt/internal/gtk/swt_adwaita_gtk_320.css"
+						"/org/eclipse/swt/internal/gtk/swt_theming_fixes_gtk_pre320.css" :
+						"/org/eclipse/swt/internal/gtk/swt_theming_fixes_gtk_320.css"
 					, true));
 	}
 
 	// Load CSS from user-defined CSS file.
 	String additionalCSSPath = System.getProperty("org.eclipse.swt.internal.gtk.cssFile");
 	if (GTK.GTK_VERSION >= OS.VERSION(3, 14, 0) && additionalCSSPath != null){
-		// Warning: gtk css syntax changed in 3.20. If you load custom css, it could break things depending on gtk version on system.
+		// Warning:
+		// - gtk css syntax changed in 3.20. If you load custom css, it could break things depending on gtk version on system.
+		// - Also, a lot of custom css/themes are buggy and may result in additional console warnings.
 		combinedCSS.append(load.apply(additionalCSSPath, false));
 	}