Bug 188408 - [GTK] Web Inspector: Checkboxes in Inspector elements panel are rendered incorrect
Summary: [GTK] Web Inspector: Checkboxes in Inspector elements panel are rendered inco...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-08 08:17 PDT by j.tan
Modified: 2018-10-24 07:43 PDT (History)
4 users (show)

See Also:


Attachments
Screenshot of elements panel (12.17 KB, image/png)
2018-08-08 08:17 PDT, j.tan
no flags Details
Screenshot of checkbox clickable area and visible position (1.00 KB, image/png)
2018-10-24 07:43 PDT, j.tan
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description j.tan 2018-08-08 08:17:56 PDT
Created attachment 346770 [details]
Screenshot of elements panel

Checkboxes in front of style rules look to be too big and subsequently clipped.
Scrolling, clicking or hovering the cursor over them changes the appearance, but they still look bad.
Comment 1 Michael Catanzaro 2018-08-08 09:05:46 PDT
We should probably not try to use GTK+ theme for checkboxes that are this small.

Carlos Eduardo, what do you think?
Comment 2 Carlos Bentzen 2018-08-13 15:23:19 PDT
(In reply to Michael Catanzaro from comment #1)
> We should probably not try to use GTK+ theme for checkboxes that are this
> small.
> 
> Carlos Eduardo, what do you think?

Hi Michael! I took a look at this on Friday but didn't have time to comment here.

Yes, this is another problem with GTK+ theme rendering bigger areas than the page style asks for.

In the inspector the mentioned checkbox is 10x10, but GTK+ minimumSize() is 24x16.

It's one of the things in the way in a patch I was writing on the spinbuttons too. It's in a sensible area (RenderThemeGadget::render()) that could cause lots of regressions or tests needed to be rebased.

I tried to not draw with GTK+ (using RenderThemeGtk::isControlStyled()) but then the engine simply does not draw any checkbox at all. Not good.

I'm trying to come up with a good solution but at a first glance I couldn't find how to solve it trivially.
Comment 3 Michael Catanzaro 2018-08-13 17:43:01 PDT
(In reply to Carlos Eduardo Ramalho from comment #2)
> I tried to not draw with GTK+ (using RenderThemeGtk::isControlStyled()) but
> then the engine simply does not draw any checkbox at all. Not good.

Hm, there should be a fallback style (used e.g. for WPE)
Comment 4 Carlos Bentzen 2018-08-14 11:55:44 PDT
(In reply to Michael Catanzaro from comment #3)
> Hm, there should be a fallback style (used e.g. for WPE)

WPE uses a different Theme API (guarded by USE(NEW_THEME)), where drawing of widgets is done directly via GraphicsContext calls. Hence no need for a widget library. This code is in WebCore/platform/wpe/ThemeWPE.cpp

I was thinking of migrating the GTK theme to this new model to enable threaded painting in WebKitGTK+ (see discussion in bug 186840) but it's a lot of work... Not sure if it's the best thing to do in the long-term. Currently Mac and WPE ports use NEW_THEME.

What do you think about this, Carlos Garcia?

Not sure if this discussion should be placed in this bug.. If a follow-up discussion is worth I could open another bug.
Comment 5 Carlos Bentzen 2018-08-14 11:56:10 PDT
(In reply to Michael Catanzaro from comment #3)
> Hm, there should be a fallback style (used e.g. for WPE)

WPE uses a different Theme API (guarded by USE(NEW_THEME)), where drawing of widgets is done directly via GraphicsContext calls. Hence no need for a widget library. This code is in WebCore/platform/wpe/ThemeWPE.cpp

I was thinking of migrating the GTK theme to this new model to enable threaded painting in WebKitGTK+ (see discussion in bug 186840) but it's a lot of work... Not sure if it's the best thing to do in the long-term. Currently Mac and WPE ports use NEW_THEME.

What do you think about this, Carlos Garcia?

Not sure if this discussion should be placed in this bug.. If a follow-up discussion is worth I could open another bug.
Comment 6 Carlos Garcia Campos 2018-08-16 01:31:49 PDT
(In reply to Carlos Eduardo Ramalho from comment #5)
> (In reply to Michael Catanzaro from comment #3)
> > Hm, there should be a fallback style (used e.g. for WPE)
> 
> WPE uses a different Theme API (guarded by USE(NEW_THEME)), where drawing of
> widgets is done directly via GraphicsContext calls. Hence no need for a
> widget library. This code is in WebCore/platform/wpe/ThemeWPE.cpp

I don't think NEW_THEME imposes to use graphics context, it's the same with the other path, we get a graphics context too. The problem is that the GTK+ theme api renders in a cairo context that we get from the graphics context.

> I was thinking of migrating the GTK theme to this new model to enable
> threaded painting in WebKitGTK+ (see discussion in bug 186840) but it's a
> lot of work... Not sure if it's the best thing to do in the long-term.
> Currently Mac and WPE ports use NEW_THEME.
> 
> What do you think about this, Carlos Garcia?

I don't understand how using NEW_THEME would allow us to enable threaded painting. Could you elaborate?

> Not sure if this discussion should be placed in this bug.. If a follow-up
> discussion is worth I could open another bug.
Comment 7 j.tan 2018-10-24 07:43:49 PDT
Created attachment 353035 [details]
Screenshot of checkbox clickable area and visible position

Added another attachment which shows the clickable area (shown in white) only partially overlaps the place where the checkbox is drawn. This means that a part of the visible checkbox does not toggle the checkbox when clicked.

Select some text around the checkbox to reproduce the screenshot.