Bug 175067

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.

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.

(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.

(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?

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

(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.

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

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.

Created attachment 346226 [details] WIP

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.

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.

(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+.

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

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

(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?

(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

Yes.