Closed
Bug 1027138
Opened 10 years ago
Closed 10 years ago
--enable-default-toolkit=cairo-gtk3 requires Gtk+ 3.6
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox33 fixed)
RESOLVED
FIXED
mozilla33
Tracking | Status | |
---|---|---|
firefox33 | --- | fixed |
People
(Reporter: glandium, Assigned: stransky)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
Because of gtk_menu_button_new.
Same concern as bug 1027000. We may or may not want that depending on what systems we target.
Reporter | ||
Comment 1•10 years ago
|
||
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.
Reporter | ||
Comment 2•10 years ago
|
||
RHEL7/Centos7 have 3.8.
Reporter | ||
Comment 3•10 years ago
|
||
Ubuntu 12.04 (currently LTS, and what we have on test slaves) has 3.4, 13.04 has 3.6,
Reporter | ||
Comment 4•10 years ago
|
||
Martin, could you take a look at this? Backing out bug 984075 is not really thrilling, as it breaks arrows.
Assignee | ||
Comment 6•10 years ago
|
||
We can use combo box arrow as a workaround for gtk 3.4 system - seems to work as expected.
Assignee | ||
Comment 7•10 years ago
|
||
Karl, what about this one? It's not ideal but it works as a workaround.
Attachment #8445869 -
Flags: review?(karlt)
Reporter | ||
Comment 8•10 years ago
|
||
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•10 years ago
|
||
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•10 years ago
|
||
(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.
Assignee | ||
Comment 11•10 years ago
|
||
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.
Attachment #8445869 -
Attachment is obsolete: true
Attachment #8445869 -
Flags: review?(karlt)
Attachment #8446421 -
Flags: review?(karlt)
Comment 12•10 years ago
|
||
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.)
Attachment #8446421 -
Flags: review?(karlt) → review+
Assignee | ||
Comment 13•10 years ago
|
||
Thanks. There's the one with updated comment.
No need to run try as it modifies gtk3 only file gtk3drawing.c.
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 14•10 years ago
|
||
Actually, please do a try first. With this applied: https://bugzilla.mozilla.org/attachment.cgi?id=8444892
Keywords: checkin-needed
Assignee | ||
Comment 15•10 years ago
|
||
ok, there's the try here - https://tbpl.mozilla.org/?tree=Try&rev=3d843dc39e4c
Reporter | ||
Comment 16•10 years ago
|
||
(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.
Keywords: checkin-needed
Updated•10 years ago
|
Attachment #8446421 -
Attachment is obsolete: true
Reporter | ||
Comment 18•10 years ago
|
||
(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.
Keywords: checkin-needed
Comment 19•10 years ago
|
||
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.
Keywords: checkin-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... :\
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox33:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•