Bug 175067 – [GTK] Spin buttons are drawn too large

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Alberto Lopez Perez]

This bug is has been moved from https://bugzilla.gnome.org/show_bug.cgi?id=785578

Original report:

"
This happened in the last few days, with an update to the site.

See screenshots comparing the result in epiphany and chrome, epiphany's number controls are too big hiding the result area.
"

[Carlos Alberto Lopez Perez]

Here are some screen-shots showing how chrome and webkitgtk renders the generate random number table at https://www.random.org/

 * chrome: https://people.igalia.com/clopez/wkbug/175067/chrome.png
 * webkitgtk before applying fix for bug 173572 : https://people.igalia.com/clopez/wkbug/175067/webkitgtk_2.16.3.png
 * webkitgtk after applying fix for bug 173572 : https://people.igalia.com/clopez/wkbug/175067/webkitgtk_2.16.6.png


As you can see, after fix for bug 173572 the size of the input type was not big enough to draw the value and the spin-buttons at the same time, so the value of the input type was covered (In the Max: it shows 1 instead of 100.. the "00" are covered by the spin buttons)

After fix for bug 173572 the width of the input size choose by the web designer is overriden and grow to accommodate the spin-buttons without covering the input values). So now on "Max" you an see the "100" value correctly.

However, this causes the inputs to reflow vertically as the table where they are contained is not big enough to draw both the input and the Max/Min text in the same line.

And this ends causing that the value for the generated random number to fall below the visible area of that table.

Note that this was already happening before (as you can see the value on https://people.igalia.com/clopez/wkbug/175067/webkitgtk_2.16.3.png is not draw 100% correctly, its cut at the half). But now is exaggerated more so its not even appearing.

[Carlos Alberto Lopez Perez]

And the thing is that I don't know how we can fix this.

I already felt that overriding the size of the input numbers that was choose by the web designer was maybe going to cause some issues on some sites when I proposed that solution, but I don't know how to fix this issue without doing that.

The only sane thing I thing we can do is to go back to the spin buttons that we had some years ago:

Now we have this: https://trac.webkit.org/export/217674/webkit/trunk/LayoutTests/platform/gtk/fast/forms/number/number-appearance-spinbutton-layer-expected.png

Before we had this: https://trac.webkit.org/export/148821/webkit/trunk/LayoutTests/platform/gtk/fast/forms/number/number-appearance-spinbutton-layer-expected.png


Web designers are going to keep testing their sites only with chrome/firefox/safari, and all of this browsers have small spinbuttons likes the ones we had before. Either we go back to small spinbuttons or we will breaking some sites like this one.

[Adrian Perez]

(In reply to Carlos Alberto Lopez Perez from comment #2)
> And the thing is that I don't know how we can fix this.
> 
> I already felt that overriding the size of the input numbers that was choose
> by the web designer was maybe going to cause some issues on some sites when
> I proposed that solution, but I don't know how to fix this issue without
> doing that.
> 
> The only sane thing I thing we can do is to go back to the spin buttons that
> we had some years ago:
> 
> Now we have this:
> https://trac.webkit.org/export/217674/webkit/trunk/LayoutTests/platform/gtk/
> fast/forms/number/number-appearance-spinbutton-layer-expected.png
> 
> Before we had this:
> https://trac.webkit.org/export/148821/webkit/trunk/LayoutTests/platform/gtk/
> fast/forms/number/number-appearance-spinbutton-layer-expected.png
> 
> 
> Web designers are going to keep testing their sites only with
> chrome/firefox/safari, and all of this browsers have small spinbuttons likes
> the ones we had before. Either we go back to small spinbuttons or we will
> breaking some sites like this one.

Would it be an acceptable tradeoff to use the current (new, bigger buttons)
version when the size of the field is wider than a certain threshold, and
use the other version (old, narrow buttons) for the narrow inputs? I am aware
that doing it that way makes us maintain two different ways for rendering the
inputs, but at least it wouldn't break any website, while still trying to
provide the version with the easier-to-hit button when possible.

[Michael Catanzaro]

(In reply to Adrian Perez from comment #3)
> Would it be an acceptable tradeoff to use the current (new, bigger buttons)
> version when the size of the field is wider than a certain threshold, and
> use the other version (old, narrow buttons) for the narrow inputs? I am aware
> that doing it that way makes us maintain two different ways for rendering the
> inputs, but at least it wouldn't break any website, while still trying to
> provide the version with the easier-to-hit button when possible.

That sounds like an OK option. Clearly, using extra space is not OK.

Alternatively, could we scale down the nice GTK+ spinbuttons to fit in the available space?

[Michael Catanzaro]

*** Bug 183030 has been marked as a duplicate of this bug. ***

[Carlos Bentzen]

(In reply to Michael Catanzaro from comment #4)
> (In reply to Adrian Perez from comment #3)
> > Would it be an acceptable tradeoff to use the current (new, bigger buttons)
> > version when the size of the field is wider than a certain threshold, and
> > use the other version (old, narrow buttons) for the narrow inputs? I am aware
> > that doing it that way makes us maintain two different ways for rendering the
> > inputs, but at least it wouldn't break any website, while still trying to
> > provide the version with the easier-to-hit button when possible.
> 
> That sounds like an OK option. Clearly, using extra space is not OK.

I'll try to go for this option.

> 
> Alternatively, could we scale down the nice GTK+ spinbuttons to fit in the
> available space?

I think the horizontal square buttons make it difficult to fit nicely in tiny spacing...

Also a lot of refactoring would be need to this based on the way it's currently being drawn. The most simple way would be Adrian's suggestions I think.

[Carlos Bentzen]

> The most simple way would be Adrian's suggestions I
> think.

simplest*

[Carlos Bentzen]

Created attachment 346225 [details]
Spin buttons

I went through a more conservative approach and ported the old GTK spin button to the RenderTheme{Widget,Gadget} code and made it the only option. The result is as attached here.

WDYT?

If you think we should proceed to returning to this design, I will update test expectations were needed to submit the patch for review. Below I'll send the WIP patch for the moment.

[Carlos Bentzen]

Created attachment 346226 [details]
WIP

[Michael Catanzaro]

Carlos Garcia, what do you think?

It looks like the arrow buttons are too small when the entry is larger. And even at the smallest size, they appear to be misaligned horizontally, right? I would expect the arrows to be aligned with the edge of the entry.

[Carlos Garcia Campos]

The point of our theming code is to be consistent with the rest of the platform (being GNOME the platform in this case). So, I don't think we should render the spin buttons differently, but I agree they don't work in case of small fields that are simply not allowed in the GTK+ world. So, I think we should either skip the theming when the size of the control is smaller than the minimum GTK+ spin button size for the current theme, or render a different style only when the control size is too small. It could happen that a website with spin buttons of different sizes look inconsistent, though, but I guess that's unlikely.

[Adrian Perez]

(In reply to Carlos Garcia Campos from comment #11)
> The point of our theming code is to be consistent with the rest of the
> platform (being GNOME the platform in this case). So, I don't think we
> should render the spin buttons differently, but I agree they don't work in
> case of small fields that are simply not allowed in the GTK+ world. So, I
> think we should either skip the theming when the size of the control is
> smaller than the minimum GTK+ spin button size for the current theme, or
> render a different style only when the control size is too small. It could
> happen that a website with spin buttons of different sizes look
> inconsistent, though, but I guess that's unlikely.

I agree with Carlos here. Either there is space for the buttons and they
get painted in the same way as GTK+ does, and skip them if the entry is
too small. I would rather have the spin button entry degrade to a normal
entry than having a custom widget that does not correspond to anything
available in GTK+.

[Michael Catanzaro]

I hit this today trying to configure a meeting reminder on Google Calendar.

And then again after downloading Ubuntu to test something. This example doesn't require any login: https://ubuntu.com/download/desktop/thank-you

[Michael Catanzaro]

This is fixed in trunk. The new spin buttons seem way too small tbh, but that's the opposite of the problem reported here.

[Carlos Alberto Lopez Perez]

(In reply to Michael Catanzaro from comment #14)
> This is fixed in trunk. The new spin buttons seem way too small tbh, but
> that's the opposite of the problem reported here.

It was 257299 right?

[Carlos Alberto Lopez Perez]

(In reply to Carlos Alberto Lopez Perez from comment #15)
> (In reply to Michael Catanzaro from comment #14)
> > This is fixed in trunk. The new spin buttons seem way too small tbh, but
> > that's the opposite of the problem reported here.
> 
> It was 257299 right?

r257299


r257299

[Michael Catanzaro]

Yes.