Closed Bug 978172 Opened 10 years ago Closed 10 years ago

Port GTK2 to GTK3 - missing doorhanger border

Categories

(Core :: Widget: Gtk, defect)

Product:
Core
Component:
Widget: Gtk
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla33

People

(Reporter: stransky, Assigned: stransky)

References

Details

Attachments

(1 file, 2 obsolete files)

patch for check-in
(deleted), patch
Details | Diff | Splinter Review 
Doorhangers in Gtk3 port does not have borders. It's because the border is get from GtkButton which has a transparent border color.

downstream bug - https://bugzilla.redhat.com/show_bug.cgi?id=1056317
Doorhanger background is rendered by sMozWindowBackground so it would make sense to use window border too. But the default color is too dark, compared to doorhangers gtk2 version. Plus window borders does not have light/dark border versions, just the dark one. A solution can be to use insensitive border variant.
Is there a similar widget in GTK?
Perhaps the menus are most similar?
Yes, I think menus are close to what we want here.
Attached patch patch (obsolete) (deleted) — — Splinter Review 
Unfortunately menus do not use a simple border color - it's composed from images and dashed lines in default theme. I think the closest element is a notebook tab which border I use here. Marked as TODO for further fixes.
Attachment #8388488 - Flags: review?(karlt)
This may be hard because borders in GTK3 can have all sorts of
styles through CSS, but the nsLookAndFeel code is assuming some kind of 3D
border.

GTK's default theme does have an opaque border color for button and a 3D
style.  The border color is multiplied by 1.8 to generate the light/brighter
color.

However, Adwaita uses border-image on button, with border-color: transparent.

notebook happens to have an opaque border color in both the default theme and
in Adwaita.  This seems a fairly random widget choice, if it is used for more
than just doorhangers.  It will not actually give the desired 3D feel for
buttons in the default theme.  Perhaps that doesn't matter so much, if the 3D colors
are not used much, but perhaps they are.

Adwaita has no borders on menus (border-style: none).
Is it necessary to have borders on doorhanger's then?

Menus don't need borders in Adwaita because they have a different background
color.  Should doorhangers have the "menu" background color?
(In reply to Karl Tomlinson (:karlt) from comment #5)
> This may be hard because borders in GTK3 can have all sorts of
> styles through CSS, but the nsLookAndFeel code is assuming some kind of 3D
> border.
> 
> GTK's default theme does have an opaque border color for button and a 3D
> style.  The border color is multiplied by 1.8 to generate the light/brighter
> color.
> 
> However, Adwaita uses border-image on button, with border-color: transparent.
> 
> notebook happens to have an opaque border color in both the default theme and
> in Adwaita.  This seems a fairly random widget choice, if it is used for more
> than just doorhangers.  It will not actually give the desired 3D feel for
> buttons in the default theme.  Perhaps that doesn't matter so much, if the
> 3D colors
> are not used much, but perhaps they are.

From my testing those are used from doorhangers only. The both colors (sButtonOuterLightBorder/sButtonInnerDarkBorder) are fully transparent now, without the patch.

> Adwaita has no borders on menus (border-style: none).
> Is it necessary to have borders on doorhanger's then?

Adwaita has borders on menus, but those are images+gradient. Only top menu border is missing. Emulating that for doorhangers would be difficult.

> Menus don't need borders in Adwaita because they have a different background
> color.  Should doorhangers have the "menu" background color?

Actually menu are transparent in adwaita so underlying background color is used which is GtkWindow widget (nsWindow). As for doorhangers, those are rendered as nsWindow so they have correct background color.
(In reply to Martin Stránský from comment #6)
> Adwaita has borders on menus, but those are images+gradient. Only top menu
> border is missing.

> Actually menu are transparent in adwaita so underlying background color is
> used which is GtkWindow widget (nsWindow).

The Adwaita that I'm looking at has

.menu {
    background-color: @menu_bg_color;
    color: @menu_fg_color;
    padding: 0;
    border-style: none;
}

https://git.gnome.org/browse/gnome-themes-standard/tree/themes/Adwaita/gtk-3.0/gtk-widgets.css?id=3.10.0#n2291

Can you point me at the relevant css for your theme, please?
Ahh, sorry, I missed your comment. Actually it looks like the sButtonOuterDarkBorder/sButtonOuterLightBorder colors are used in Australis to shape more elements - URL/search bar, find entry and search buttons at least. See Bug 989819.
(In reply to Karl Tomlinson (:karlt) from comment #7)
> The Adwaita that I'm looking at has
> 
> .menu {
>     background-color: @menu_bg_color;
>     color: @menu_fg_color;
>     padding: 0;
>     border-style: none;
> }
> 
> https://git.gnome.org/browse/gnome-themes-standard/tree/themes/Adwaita/gtk-3.
> 0/gtk-widgets.css?id=3.10.0#n2291
> 
> Can you point me at the relevant css for your theme, please?

Ahh, sorry, I was mistaken by menu bars. Menu itself has a solid background color (1,1,1,1).
In case you don't know, there is now a popover widget in GTK3. It is similar to your "doorhanger" widget, at least from non-programmer perspective. Maybe you could reuse it or its theming?
Reference:
http://blogs.gnome.org/mclasen/2014/03/18/new-in-gtk-3-12-popovers/
https://developer.gnome.org/gtk3/3.11/GtkPopover.html
(In reply to aliaspam from comment #11)
> Maybe you could reuse it or its theming?

Thank you.  Yes, that would be the ideal solution.
Hm, unfortunately they're available in Gnome 3.12 only.
There are also content widgets that use those transparent colors from adwaita. See bug 1022123.
(In reply to Karl Tomlinson (needinfo?:karlt) from comment #12)
> (In reply to aliaspam from comment #11)
> > Maybe you could reuse it or its theming?
> 
> Thank you.  Yes, that would be the ideal solution.

We still need to get the nsLookAndFeel colors fixed regardless.
A crazy idea would be to take those border-images, and use the dominant/average color.

Arguably, for pure widgets like UI buttons, we /should/ simply reuse the gtk css rules. For custom things like the doorhangers, we obviously need colors the suit the theme.
(In reply to Mike Hommey [:glandium] from comment #17)
> A crazy idea would be to take those border-images, and use the
> dominant/average color.

Ideally the theme would specify a border-color and let the border-image override that. How realistic would it be to get the default theme updated to that effect?
(In reply to Dão Gottwald [:dao] from comment #18)
> Ideally the theme would specify a border-color and let the border-image
> override that. How realistic would it be to get the default theme updated to
> that effect?

I guess that can be argued with them. That won't fix the issue for people not using the hypothetical updated theme.

Note, considering my experiments with a gtk.css file, I was afraid it wouldn't work, because adding a border-color in gtk.css makes gtk use the border, without changing the value for border-image. But it turns out gtk is apparently not really doing the cascading part of CSS, because defining both border-image and border-color in gtk.css makes it use the image.
> adding a border-color in gtk.css makes gtk use the border

use the (border) color (instead of the image).
Unfortunately, a cursory look shows that there are transparent border-colors with border-images in more themes than Adwaita. I found some in clearlooks-phenix-theme and mate-themes. There may be more.
Yes, even if we can convince some themes to play more friendly for our needs, we will probably need some sensible fallback for missing colors.
Apparently Adwaita will go through quite some changes in the 3.14 cycle (see http://jimmac.musichall.cz/blog/2014-06-14-adwaita-3-14/ ), such as a reduction in image assets and becoming GTK's built-in default theme. Maybe this will improve the situation.
That's a good news. But we need to decide if we support other themes (I think yes) and how - the image borders can be used by any other themes so we would need some workaround here. I like the idea in comment 17 - to render gtk element and read border color from pixbuf.
Comment on attachment 8388488 [details] [diff] [review]
patch

I think it is fine to change the widget from a button, if these colors are
used mainly for frames than buttons, which seems to be the case based on
https://developer.mozilla.org/en-US/docs/Web/CSS/color_value

However, in doing that, I'd like the variables renamed so they are not
called sButton*.

Swapping PRELIGHT and NORMAL in this patch is confusing, and I assume it does
nothing for notebooks.

Currently PRELIGHT maps to hover in the builtin themes gtk-default.css, but if
this is changed to a different widget, then PRELIGHT will have no effect.

PRELIGHT probably wasn't quite right anyway because what we really wanted was
the 1.8 multiplier on the lightness and saturation of the color, but this is
only appropriate for themes using GTK_BORDER_STYLE_INSET/OUTSET/GROVE/RIDGE,
such as the builtin theme.  Adwaita has a solid border and so both colors
should be the same.  Which approach to use can be determined from
gtk_style_context_get_style_property(,GTK_STYLE_PROPERTY_BORDER_STYLE,), but
you can save this for a follow up and just use solid for now if you like.

I'd prefer to use a GtkFrame over a GtkNotebook.  It is really the frame that
we want, and notebook kind of emulates the frame.
Attachment #8388488 - Flags: review?(karlt) → review-
Attached patch patch, v.2 (obsolete) (deleted) — — Splinter Review 
Thanks, so something like that? I'd like to leave the border styles for later.
Attachment #8388488 - Attachment is obsolete: true
Attachment #8444408 - Flags: review?(karlt)
Comment on attachment 8444408 [details] [diff] [review]
patch, v.2

Yes, thanks.

>+    GtkWidget *frame = gtk_frame_new("M");

Please just use nullptr for the label parameter.
Attachment #8444408 - Flags: review?(karlt) → review+
Attached patch patch for check-in (deleted) — — Splinter Review 
Thanks!
Attachment #8444408 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/6d06526b4d4a
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Blocks: 1034064
No longer blocks: 1034064
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: