Bug 161735 – [GTK] Scrollbar too large

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Milan Crha]

I'm going to attach two screenshots which show that the WebKit2 scrollbars are drown too large for me.

[Milan Crha]

Created attachment 288257 [details]
devhelp.png

This is from DevHelp, which uses stock webkitgtk4-2.12.2-2.fc24.x86_64

[Milan Crha]

Created attachment 288258 [details]
evo.png

This is from the evolution, the message content uses WebKit2. It's self-build git master evolution linking to 2.13.90 WebKitGTK+ release with only one applied patch, from bug #161605 comment #10.

[Carlos Garcia Campos]

What theme is that? Does the same happen in foreign drawing gtk demo?

[Milan Crha]

It's BlueMenta. Clearlooks and Adwaita draw it correctly.

I do not know what you mean with "foreign drawing gtk demo", but when you look on both screenshots, then they also contain gtk+ widgets with gtk+ scrollbars, which has correct width (in devhelp the vertical scrollbar in the middle of the screenshot).

[Carlos Garcia Campos]

I mean run gtk3-demo and load the foreign drawing one from the tree view list. The way widgets are rendered in WebKit (and foreign drawing demo) is different to how real widget are drawn.

[Milan Crha]

Created attachment 288259 [details]
frg.png

Here's how it looks like in the gtk3-demo->Foreign drawing

[Carlos Garcia Campos]

Created attachment 288568 [details]
Patch

[Milan Crha]

Created attachment 288570 [details]
patched.png

The patch makes it better, though not pixel precise (see the width is slightly bigger that the native scrollbar's above it). Also note of the different colors of the buttons with the arrows. While the native scrollbar uses black color for the triangle, WebKitGTK+ uses white color. The disabled scrollbar button looks the same, color speaking.

[Carlos Garcia Campos]

It's very difficult to match every single theme pixel by pixel. This is the best I can do.

[Milan Crha]

(In reply to comment #9)
> It's very difficult to match every single theme pixel by pixel. This is the
> best I can do.

That's okay, I mentioned it only because noticed it. To be honest, I think it's more a problem of the theme, not providing proper numbers.

Similar problem can apply for the colors, though it's odd, if you use the gtk+ paint functions, where I would expect doing gtk+ the right thing for you (unless you'd be dependent on GtkCSSNode, which is not going to be publicized).

[Carlos Garcia Campos]

(In reply to comment #10)
> (In reply to comment #9)
> > It's very difficult to match every single theme pixel by pixel. This is the
> > best I can do.
> 
> That's okay, I mentioned it only because noticed it. To be honest, I think
> it's more a problem of the theme, not providing proper numbers.

I'm sorry if I sounded rude, it was not my intention.

> Similar problem can apply for the colors, though it's odd, if you use the
> gtk+ paint functions, where I would expect doing gtk+ the right thing for
> you (unless you'd be dependent on GtkCSSNode, which is not going to be
> publicized).

We use all the GTK+ CSS nodes machinery, but not exactly the same way than GTK+, we would need to write GTK+ inside WebKit to get the same. The buttons color is probably because we are using gtk_render_arrow to render the arrows, instead of using an image CSS node. I'll look at it, but in a different bugs, because it's a different issue.

[Milan Crha]

(In reply to comment #11)
> I'm sorry if I sounded rude, it was not my intention.

No problem.

> I'll look at it, but in a different bugs, because it's a different issue.

Okay, thanks.

[Michael Catanzaro]

Comment on attachment 288568 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=288568&action=review

> Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:531
> +    static std::unique_ptr<RenderThemeGadget> sliderGadget;

You overwrite it each time you use it, so it doesn't make sense for it to be static, right? Fix this please.

[Carlos Garcia Campos]

Comment on attachment 288568 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=288568&action=review

>> Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:531
>> +    static std::unique_ptr<RenderThemeGadget> sliderGadget;
> 
> You overwrite it each time you use it, so it doesn't make sense for it to be static, right? Fix this please.

Good catch! this is just wrong, I wanted to copy paste the type from the create method in RenderThemeGadget.h but included the static in the selection, thanks!

[Carlos Garcia Campos]

Committed r205853: <http://trac.webkit.org/changeset/205853>