Skip to content

[GTK4] Fix scrollbar warnings when using Breeze theme #2424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ptziegler
Copy link
Contributor

Trying to open e.g. a shell (or any widget with a scrollbar) produces the following warning:

GtkGizmo (...) (slider) reported min height -2, but sizes must be >= 0

This is a bug with the KDE theme:
https://bugs.kde.org/show_bug.cgi?id=486766

More specifically, the theme is reporting the wrong size for the scroll-bar (6px, as opposed to 8px). To fix this, adjust the CSS theme to define a minimum size for the slider.

To reproduce, run Snippet1 with GTK4 and the Breeze theme.

Trying to open e.g. a shell (or any widget with a scrollbar) produces
the following warning:
> GtkGizmo (...) (slider) reported min height -2, but sizes must be >= 0

This is a bug with the KDE theme:
https://bugs.kde.org/show_bug.cgi?id=486766

More specifically, the theme is reporting the wrong size for the
scroll-bar (6px, as opposed to 8px). To fix this, adjust the CSS theme
to define a minimum size for the slider.

To reproduce, run Snippet1 with GTK4 and the Breeze theme.
Copy link
Contributor

Test Results

   491 files   -    55     491 suites   - 55   27m 43s ⏱️ - 1m 13s
 4 388 tests  -    37   4 373 ✅  -    35   15 💤  - 2  0 ❌ ±0 
13 123 runs   - 3 623  13 004 ✅  - 3 615  119 💤  - 8  0 ❌ ±0 

Results for commit a512ad6. ± Comparison against base commit a641cdb.

This pull request removes 37 tests.
AllGTKTests Test_GtkConverter ‑ test_HeuristicASCII_dollarSign
AllGTKTests Test_GtkConverter ‑ test_HeuristicASCII_emptyString
AllGTKTests Test_GtkConverter ‑ test_HeuristicASCII_letterA
AllGTKTests Test_GtkConverter ‑ test_HeuristicASCII_letters
AllGTKTests Test_GtkConverter ‑ test_HeuristicUTF16LE_null
AllGTKTests Test_GtkConverter ‑ test_HeuristicUTF16_AsciiLetters
AllGTKTests Test_GtkConverter ‑ test_HeuristicUTF16_Asciiletter
AllGTKTests Test_GtkConverter ‑ test_HeuristicUTF16_LotsOfLetters
AllGTKTests Test_GtkConverter ‑ test_HeuristicUTF16_letter
AllGTKTests Test_GtkConverter ‑ test_HeuristicUTF16_letters
…

@akurtakov
Copy link
Member

This PR is going totally in the wrong direction IMO. SWT does whole lot of things to allow components to be smaller (down to 0,0) via SwtFixed that setting min size bigger for the sake of one theme feels totally wrong, it might even have unpredictable effects in some places.
I can be convinced to accept a modified version where this brezze specific css is moved to a separate breeze css file and is applied only when running theme is breeze.

@ptziegler
Copy link
Contributor Author

@akurtakov I see your point. Having a dedicated CSS file for Breeze would be possible, but I'd like to avoid such a hard dependency if at all possible. How were theming issues handled in the past?

@akurtakov
Copy link
Member

Most often it was actual issue in SWT - not taking account of padding, border, etc. and the fix was to make SWT query one more Gtk property and use it in calculations. For actual issues in themes e.g. Oxygen was quite problematic one it ended up with "Bugs that are reproducible only with themes other than Adwaita will be given a lower priority (or may not be fixed at all), compared to bugs which are reproducible on the target environments listed above." in https://eclipse.dev/eclipse/development/plans/eclipse_project_plan_4_37.xml#target_environments or https://github.com/eclipse-equinox/equinox/blob/80c4668d7baeeee85369cb3b671f5b5876e372bd/features/org.eclipse.equinox.executable.feature/library/eclipse.c#L500 .
I don't remember a case where a workaround for broken theme was applied for all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants