1278108 - <input type="range"> does not display on Firefox 46 with GTK+ 3.20

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[c3526836]

Attachment: screenshot of test with focus on range element

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160503215307

Steps to reproduce:

Load any page with an <input type="range"> slider element, on an Archlinux system with default theming (Adwaita?).


Actual results:

Element space is consumed, but nothing but transparency is drawn instead.

Although when you click on the element in order to create focus, an outline is visible (see screenshot).


Expected results:

A slider element should have been visible and usable.

Comment 1

[c3526836]

Not that I could see and use this element normally in previous versions.

Comment 2

[c3526836]

>Not that I could see and use this element normally in previous versions.

Note that*

Comment 3

[Martin Stránský [:stransky] (ni? me)]

yes I see that.

Comment 4

[Bill Gianopoulos [:WG9s]]

Changing bug this blocks as this works no differently under gtk 3.18.

Comment 5

[Karl Tomlinson (:karlt)]

Modulo bug 1264170, this renders as expected for me, with GTK 3.18.7 Adwaita.
With what 3.18 theme are you seeing this bug, Bill?

Comment 6

[Bill Gianopoulos [:WG9s]]

(In reply to Karl Tomlinson (ni?:karlt) from comment #5)
> Modulo bug 1264170, this renders as expected for me, with GTK 3.18.7 Adwaita.
> With what 3.18 theme are you seeing this bug, Bill?

Silly me misread the attachment.  that was a Screenshot of the failure.  I tried the data URL and it does indeed work under 3.18.  My bad.

Comment 7

[Martin Stránský [:stransky] (ni? me)]

The GtkScale seems to have a quite complicated CSS scheme:
https://developer.gnome.org/gtk3/stable/GtkScale.html

We may need to find some subset which needs to be implemented by Gecko.

Comment 8

[Karl Tomlinson (:karlt)]

(In reply to Martin Stránský from comment #7)
> The GtkScale seems to have a quite complicated CSS scheme:
> https://developer.gnome.org/gtk3/stable/GtkScale.html
> 
> We may need to find some subset which needs to be implemented by Gecko.

Yes. AFAIK we don't need the marks, value, or fill.
has-origin defaults to true, so maybe highlight should be drawn.
I don't think we have in the past though, so that may be an existing issue.

Comment 9

[Martin Stránský [:stransky] (ni? me)]

The range itself can be rendered quite easily - we just need to use nodes like we do with scrollbars. 

A different part is to render it correctly - as Gtk does that. The thumb uses negative margin and overlaps the through in Adwaita style and size of the through itself is difficult to get - I didn't find a way how to derive it.

Comment 10

[Martin Stránský [:stransky] (ni? me)]

Attachment: patch

There's a basic patch for it. The scale is not rendered as well as native scales in gtk3.20 (it's a bit bigger) but it's the best I can produce now. We may also fix focus ring of the whole element (or don't draw it and draw only focus around the track).

Comment 11

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8763063 [details] [diff] [review]
patch

The focus is rendered twice. Once in moz_gtk_scale_paint() around the track (the small one) and there's also big focus around whole element - I have no idea where it comes from.

Comment 12

[Bill Gianopoulos [:WG9s]]

This does not fix the issue with Gtk 3.18.9 under fedora23.

Comment 13

[Martin Stránský [:stransky] (ni? me)]

(In reply to Bill Gianopoulos [:WG9s] from comment #12)
> This does not fix the issue with Gtk 3.18.9 under fedora23.

That's possible, I target it to 3.20.

Comment 14

[Bill Gianopoulos [:WG9s]]

I did just read back and the original reporter did say he does not know of any previous version where this ever worked, so fixing it for the new stuff might be sufficient.  I thought this was a regression.  I am doing a gtk2 build to run this testcase against.

Comment 15

[Bill Gianopoulos [:WG9s]]

(In reply to Martin Stránský from comment #13)
> (In reply to Bill Gianopoulos [:WG9s] from comment #12)
> > This does not fix the issue with Gtk 3.18.9 under fedora23.
> 
> That's possible, I target it to 3.20.

These are the errors I get under gtk2.18:

(firefox:2728): Gtk-CRITICAL **: gtk_widget_get_style_context: assertion 'GTK_IS_WIDGET (widget)' failed

(firefox:2728): Gtk-CRITICAL **: gtk_style_context_get_state: assertion 'GTK_IS_STYLE_CONTEXT (context)' failed

(firefox:2728): Gtk-CRITICAL **: gtk_style_context_get_direction: assertion 'GTK_IS_STYLE_CONTEXT (context)' failed

(firefox:2728): Gtk-CRITICAL **: gtk_render_slider: assertion 'GTK_IS_STYLE_CONTEXT (context)' failed

Comment 16

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #15)
> (In reply to Martin Stránský from comment #13)
> > (In reply to Bill Gianopoulos [:WG9s] from comment #12)
> > > This does not fix the issue with Gtk 3.18.9 under fedora23.
> > 
> > That's possible, I target it to 3.20.
> 
> These are the errors I get under gtk2.18:

I meant gtk3.18.

Comment 17

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #16)
> (In reply to Bill Gianopoulos [:WG9s] from comment #15)
> > (In reply to Martin Stránský from comment #13)
> > > (In reply to Bill Gianopoulos [:WG9s] from comment #12)
> > > > This does not fix the issue with Gtk 3.18.9 under fedora23.
> > > 
> > > That's possible, I target it to 3.20.
> > 
> > These are the errors I get under gtk2.18:
> 
> I meant gtk3.18.

It would seem the issue is in moz_gtk_scale_paint were we get the following bizarre code which tests for gtk3 version being 3.20 or grater but then duplicates the code in both cases:

  if (gtk_check_version(3, 20, 0) != nullptr) {
      widget = (flags == GTK_ORIENTATION_HORIZONTAL) ?
                         MOZ_GTK_SCALE_HORIZONTAL :
                         MOZ_GTK_SCALE_VERTICAL;
  } else {
      widget = (flags == GTK_ORIENTATION_HORIZONTAL) ?
                         MOZ_GTK_SCALE_TROUGH_HORIZONTAL :
                         MOZ_GTK_SCALE_TROUGH_VERTICAL;
  }

Comment 18

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #14)
> I did just read back and the original reporter did say he does not know of
> any previous version where this ever worked, so fixing it for the new stuff
> might be sufficient.  I thought this was a regression.  I am doing a gtk2
> build to run this testcase against.

OK.  It took some doing because bug 1278236 broke building with the cairo-gtk2 toolkit.  But, I finally got a gtk2 build to work and this does work under gtk2, so it is a valid regression bug.

Comment 19

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #17)
> (In reply to Bill Gianopoulos [:WG9s] from comment #16)
> > (In reply to Bill Gianopoulos [:WG9s] from comment #15)
> > > (In reply to Martin Stránský from comment #13)
> > > > (In reply to Bill Gianopoulos [:WG9s] from comment #12)
> > > > > This does not fix the issue with Gtk 3.18.9 under fedora23.
> > > > 
> > > > That's possible, I target it to 3.20.
> > > 
> > > These are the errors I get under gtk2.18:
> > 
> > I meant gtk3.18.
> 
> It would seem the issue is in moz_gtk_scale_paint were we get the following
> bizarre code which tests for gtk3 version being 3.20 or grater but then
> duplicates the code in both cases:
> 
>   if (gtk_check_version(3, 20, 0) != nullptr) {
>       widget = (flags == GTK_ORIENTATION_HORIZONTAL) ?
>                          MOZ_GTK_SCALE_HORIZONTAL :
>                          MOZ_GTK_SCALE_VERTICAL;
>   } else {
>       widget = (flags == GTK_ORIENTATION_HORIZONTAL) ?
>                          MOZ_GTK_SCALE_TROUGH_HORIZONTAL :
>                          MOZ_GTK_SCALE_TROUGH_VERTICAL;
>   }

Those are not the same are they?  What's wrong with your eyes Bill?

Comment 20

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #16)
> (In reply to Bill Gianopoulos [:WG9s] from comment #15)
> > (In reply to Martin Stránský from comment #13)
> > > (In reply to Bill Gianopoulos [:WG9s] from comment #12)
> > > > This does not fix the issue with Gtk 3.18.9 under fedora23.
> > > 
> > > That's possible, I target it to 3.20.
> > 
> > These are the errors I get under gtk2.18:
> 
> I meant gtk3.18.

I also have an old 32-bit laptop that does not have enough memory to run Gnome3 so I am running fedora24 mate-compiz spin on that using the BlueMenta theme.  This patch does not work for that case either.

Comment 21

[Martin Stránský [:stransky] (ni? me)]

Attachment: patch

Fixed one, works also on pre3.20.

Comment 22

[Bill Gianopoulos [:WG9s]]

Attachment: Same patch, just fixing bitrot

Nothing new here.  Just updating so it applies on current mozilla-central.

Comment 23

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #22)
> Created attachment 8776271 [details] [diff] [review]
> Same patch, just fixing bitrot
> 
> Nothing new here.  Just updating so it applies on current mozilla-central.

I give up on trying to keep patches here able to apply.  Builds including this patch are available at http://www.wg9s.com/mozilla/firefox/ and the patch I ma using which applies at current trunk will be available here http://www.wg9s.com/mozilla/firefox/patches/scale.patch

Comment 24

[Karl Tomlinson (:karlt)]

Comment on attachment 8764902 [details] [diff] [review]
patch

Looks good, thanks.

> moz_gtk_get_scale_metrics(GtkOrientation orient, gint* scale_width,

>+  WidgetNodeType widget = (orient == GTK_ORIENTATION_HORIZONTAL) ?
>+                           MOZ_GTK_SCALE_HORIZONTAL :
>+                           MOZ_GTK_SCALE_VERTICAL;

>+      GtkStyleContext* style = ClaimStyleContext(widget);
>+      gtk_style_context_get(style, gtk_style_context_get_state(style),
>+                            "min-width", scale_width,
>+                            "min-height", scale_height,
>+                            nullptr);
>+      ReleaseStyleContext(style);

I suspect the trough, and maybe even the slider should also be considered
here, but what you have here should work for Adwaita, so let's go with what
you have for now at least.

Comment 26

[Martin Stránský [:stransky] (ni? me)]

Attachment: patch for check-in

Thanks, there's a patch for check-in.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b40290a5a7f9

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef6de41494f3
Implement <input type="range"> on gtk 3.20. r=karlt

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ef6de41494f3

Comment 29

[flying sheep]

could you please backport this to 48–50? it’s pretty bad to be unable to see UI elements…

Comment 30

[Martin Stránský [:stransky] (ni? me)]

(In reply to flying sheep from comment #29)
> could you please backport this to 48–50? it’s pretty bad to be unable to see
> UI elements…

Is there any real website which uses that range UI element? Maybe some router admin UI something?

Comment 31

[flying sheep]

It's a regression bug, not a feature request, so why shouldn't it be in a bugfix release?

Comment 32

[Karl Tomlinson (:karlt)]

It's a regression in GTK, not a regression in Firefox.

Uplifting workarounds for GTK regressions are evaluated based on the need and difficultly.

Comment 33

[flying sheep]

This takes the sting off it, true. We can't be expected to work around other products’ breakages the week they occur in.

But even without putting it into the current version, Firefox 51 is something I'd expect a new feature to end up in, not a fix.

A HTML 5 element not being usable on Linux until 2017-01-24 is pretty bad.

Comment 34

[Bill Gianopoulos [:WG9s]]

(In reply to flying sheep from comment #33)
> This takes the sting off it, true. We can't be expected to work around other
> products’ breakages the week they occur in.
> 
> But even without putting it into the current version, Firefox 51 is
> something I'd expect a new feature to end up in, not a fix.
> 
> A HTML 5 element not being usable on Linux until 2017-01-24 is pretty bad.

Well this dialog really does not belong int he bug, but it would seem to me there are 2 classes of Linux users, those who run nightlies and this is fixed, and those who run the provided by their linux distro in which case you should go through them to add this patch to the version they distribute.