1288413 - Move GtkTreeView and co. to WidgetCache

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

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

Let's move other widgets to WidgetCache and use them in other widget code.

Comment 1

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

Attachment: patch

There's a patch for it, Thanks.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9dd605031300

Comment 2

[Andrew Comminos [:acomminos]]

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

Review of attachment 8775110 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/gtk/WidgetStyleCache.cpp
@@ +286,5 @@
> +static GtkWidget*
> +CreateTreeHeaderSortArrowWidget()
> +{
> +  /* TODO, but it can't be NULL */
> +  return gtk_button_new();

Any reason we can't just parent this into the window container with AddToWindowContainer? It's not appropriately styled regardless, I don't think being a descendant on GtkWindow will make a difference. The use of a floating reference saves some code.

@@ +519,1 @@
>      default:

According to the CSS node documentation for GtkTreeView, header cells should be a descendant of the "header" CSS node;

> GtkTreeView has a main CSS node with name treeview and style class .view. It has a subnode with name header, which is the 
> parent for all the column header widgets' CSS nodes. For rubberband selection, a subnode with name rubberband is used.

Should we be using this for MOZ_GTK_TREE_HEADER_CELL?

::: widget/gtk/gtk3drawing.cpp
@@ +1174,5 @@
>                            rect->width - 2 * xthickness,
>                            rect->height - 2 * ythickness);
> +    ReleaseStyleContext(style_tree);
> +
> +    style = ClaimStyleContext(MOZ_GTK_SCROLLED_WINDOW, direction);

Why do we need to render the frame with MOZ_GTK_SCROLLED_WINDOW here where we didn't before?

Comment 3

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

Attachment: patch v.2

Thanks, there's the updated one with button integrated to our widget tree.


> @@ +519,1 @@
> >      default:
> 
> According to the CSS node documentation for GtkTreeView, header cells should
> be a descendant of the "header" CSS node;
> 
> > GtkTreeView has a main CSS node with name treeview and style class .view. It has a subnode with name header, which is the 
> > parent for all the column header widgets' CSS nodes. For rubberband selection, a subnode with name rubberband is used.
> 
> Should we be using this for MOZ_GTK_TREE_HEADER_CELL?

It means we have to construct whole CSS tree for this widget which also includes MOZ_GTK_TREEVIEW_VIEW, MOZ_GTK_TREEVIEW_EXPANDER clases. We also need to change button rendering routine to accept style instead of widget for that and update combobox rendering code.

I'd rather leave that for separated path, it's marked as TODO now.
 
> ::: widget/gtk/gtk3drawing.cpp
> @@ +1174,5 @@
> >                            rect->width - 2 * xthickness,
> >                            rect->height - 2 * ythickness);
> > +    ReleaseStyleContext(style_tree);
> > +
> > +    style = ClaimStyleContext(MOZ_GTK_SCROLLED_WINDOW, direction);
> 
> Why do we need to render the frame with MOZ_GTK_SCROLLED_WINDOW here where
> we didn't before?

We get the style again because our API does not allow to hold more than one style and we need to render background first and then the frame. A workaround would me more complicated (to set state, invalidate...). The style is cached so there's no big deal here.

Comment 4

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

Comment on attachment 8775941 [details] [diff] [review]
patch v.2

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e99305c3d85

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc77bef0fabe
Move GtkTreeView widgets to WidgetCache, r=acomminos

Comment 6

[Carsten Book [:Tomcat]]

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