1027138 - --enable-default-toolkit=cairo-gtk3 requires Gtk+ 3.6

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Because of gtk_menu_button_new.

Same concern as bug 1027000. We may or may not want that depending on what systems we target.

Comment 1

[Mike Hommey [:glandium]]

Debian wheezy (current stable) has 3.4
Fedora 16 (nov 2011) had 3.2 Fedora 17 (may 2012) had 3.4. Fedora 18 (jan 2013) came with 3.6.
OpenSuSE 11.4 (mar 2011) had 3.0. 12.1 (nov 2011) had 3.2, 12.2 (jul 2012) had 3.4, 12.3 (mar 2013) has 3.6.

Comment 2

[Mike Hommey [:glandium]]

RHEL7/Centos7 have 3.8.

Comment 3

[Mike Hommey [:glandium]]

Ubuntu 12.04 (currently LTS, and what we have on test slaves) has 3.4, 13.04 has 3.6,

Comment 4

[Mike Hommey [:glandium]]

Martin, could you take a look at this? Backing out bug 984075 is not really thrilling, as it breaks arrows.

Comment 5

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

Sure.

Comment 6

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

We can use combo box arrow as a workaround for gtk 3.4 system - seems to work as expected.

Comment 7

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

Attachment: patch

Karl, what about this one? It's not ideal but it works as a workaround.

Comment 8

[Mike Hommey [:glandium]]

I'd rather not have the behaviour depend on the version of GTK you're building against, especially when most people are going to do local builds with versions greater than 3.6 and automation won't.

Comment 9

[Karl Tomlinson (:karlt)]

The problem in bug 984075 was with the arrow size, which was zero because the widget was not visible.

I suggest reverting the changes from bug 984075, and instead using gtk_widget_show(), like GtkMenuButton, to make the widget visible, so it will have a size.

That may still leave an issue because Adwaita 3.10.0 styles GtkMenuButtons differently from GtkToggleButtons so as to remove the border radius beside the menu and make the background color match the menu.
I don't know of any native menu buttons in Firefox, so that may not be a problem.
If there are native menu buttons, then gtk_widget_path_iter_set_name() may be useful.

Comment 10

[Karl Tomlinson (:karlt)]

(In reply to Karl Tomlinson (needinfo?:karlt) from comment #9)
> If there are native menu buttons, then gtk_widget_path_iter_set_name() may
> be useful.

No it won't.  names are different from types.

The GType system can be used at run time to check for availability of and make GtkMenuButtons, if this is necessary, but that can probably wait.

Comment 11

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

Attachment: patch, v.2

This one reverts the fix from Bug 984075 and adds widget_show(). I didn't see any problem on Fedora 20 with the toogle button.

Comment 12

[Karl Tomlinson (:karlt)]

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

Please explain how the arrow rendering is fixed in the checkin comment.
(It is particularly hard to work out from the patch when the fix is combined with the backout of the previous fix.)

Comment 13

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

Attachment: patch for check-in

Thanks. There's the one with updated comment. 

No need to run try as it modifies gtk3 only file gtk3drawing.c.

Comment 14

[Mike Hommey [:glandium]]

Actually, please do a try first. With this applied: https://bugzilla.mozilla.org/attachment.cgi?id=8444892

Comment 15

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

ok, there's the try here - https://tbpl.mozilla.org/?tree=Try&rev=3d843dc39e4c

Comment 16

[Mike Hommey [:glandium]]

(In reply to Martin Stránský from comment #15)
> ok, there's the try here -
> https://tbpl.mozilla.org/?tree=Try&rev=3d843dc39e4c

You're good to go.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

That looks incredibly orange-tastic to me.

Comment 18

[Mike Hommey [:glandium]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #17)
> That looks incredibly orange-tastic to me.

That's because gtk3 support is still very orange-tastic. But before that patch, it was red-tastic. And normal builds are not using the gtk3 code as mentioned in comment 13.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Fair enough. Pushed as a ride-along to fx-team.
https://hg.mozilla.org/integration/fx-team/rev/38b404a28e4e

Note that our bug marking tool doesn't like the commit message on this (due to the "revert" in it), so there's a very good chance this bug isn't going to be resolved when merged to m-c unless whoever does the merge happens to notice and does so manually.

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/38b404a28e4e
MCMerge claimed this was a security bug, which is the only reason it caught my eye while doing the merge... :\