Martin Stránský [:stransky] (ni? me) Assignee
	Description • 11 years ago

+++ This bug was initially created as a clone of Bug #627699 +++

GTK+ 3.0 and GNOME 3 are approaching and we should get Firefox ready for them.

This bug is about broken treeview rendering. See "Preferences -> Advanced -> Certificates -> Security devices" for example.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 1 • 11 years ago

This one fixes expander rendering in treeview.

	Karl Tomlinson (:karlt)
	Comment 2 • 11 years ago

Comment on attachment 782542 [details] [diff] [review]
tree-expander.patch

Looks like moz_gtk_expander_paint/MOZ_GTK_EXPANDER code is not used.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 3 • 11 years ago

> Looks like moz_gtk_expander_paint/MOZ_GTK_EXPANDER code is not used.

Yes. Filled as Bug 899460.

	Ryan VanderMeulen [:RyanVM]
	Comment 4 • 11 years ago

https://hg.mozilla.org/integration/fx-team/rev/497df39ee6ae

	Ryan VanderMeulen [:RyanVM]
	Comment 5 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/497df39ee6ae

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 6 • 11 years ago

border and button fix

	Karl Tomlinson (:karlt)
	Comment 7 • 11 years ago

Comment on attachment 785711 [details] [diff] [review]
treeview-border.patch

> moz_gtk_tree_header_cell_paint(cairo_t *cr, GdkRectangle* rect,
>                                GtkWidgetState* state,
>                                gboolean isSorted, GtkTextDirection direction)
> {
>+    gtk_tree_view_column_set_sort_indicator(gMiddleTreeViewColumn,
>+                                            isSorted);
>+
>     moz_gtk_button_paint(cr, rect, state, GTK_RELIEF_NORMAL,
>                          gTreeHeaderCellWidget, direction);

How does gtk_tree_view_column_set_sort_indicator() influence what
moz_gtk_button_paint() draws?  I understand that set_sort_indicator will
change the button in the TreeViewColumn, but I don't see that affecting the style context used in the gtk_render_ functions.

>+            *left = border.left;
>+            *right = border.right;
>+            *top = border.top;
>+            *bottom = border.bottom;
>+
>+            style = gtk_widget_get_style_context(gTreeViewWidget);
>+            gtk_style_context_add_class(style, GTK_STYLE_CLASS_VIEW);
>+
>+            *left += border.left;
>+            *right += border.right;
>+            *top += border.top;
>+            *bottom += border.bottom;

I suspect this is not what you intended.  This is adding the
gScrolledWindowWidget border twice and not using gTreeViewWidget.

However, the border of gTreeViewWidget is not used in moz_gtk_treeview_paint
so I assume we don't want it here.

Don't we want the border and padding of gScrolledWindowWidget here, and in
moz_gtk_treeview_paint?

I'd expect border and padding to be the usual thing to return from
moz_gtk_get_widget_border, and so it would be best to return that from the
default block at the end of this function.

	Karl Tomlinson (:karlt)
	Comment 8 • 11 years ago

(In reply to Karl Tomlinson (:karlt) from comment #7)
> I'd expect border and padding to be the usual thing to return from
> moz_gtk_get_widget_border, and so it would be best to return that from the
> default block at the end of this function.

Hmm, it's harder to use the default block with the gtk_style_context_add_class().
Perhaps a helper method to set a border + padding from a style context might make it easier to share code.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 9 • 11 years ago

This one adds moz_gtk_add_style_border()/moz_gtk_add_style_padding() helpers. 

I'll address the treeview style fix in another patch - it needs more work due to differences in some themes.

	Karl Tomlinson (:karlt)
	Comment 10 • 11 years ago

Comment on attachment 787772 [details] [diff] [review]
border patch

I assume we need a gtk_style_context_save/restore when adding a class.

I wonder whether it is tidier to do that if the class is added outside the helper functions and the helper functions operate on the StyleContext instead of the widget, but there are other ways to do it.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 11 • 11 years ago

Ahh, you're right, the missing save/restore. Style set is used only once so I put is outside the helper.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 12 • 11 years ago

Column button fix.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 13 • 11 years ago

FYI. last issue I'm aware of is improper expanders focus rendering. Working on that.

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 11 years ago

https://hg.mozilla.org/integration/fx-team/rev/ba7f87cc1073
https://hg.mozilla.org/integration/fx-team/rev/824ac824a539

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/ba7f87cc1073
https://hg.mozilla.org/mozilla-central/rev/824ac824a539

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 16 • 11 years ago

Don't use treeview widget flags, as we do for gtk2.

	Ryan VanderMeulen [:RyanVM]
	Comment 17 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/5eecb74f17da

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/5eecb74f17da