Closed Bug 1207310 Opened 4 years ago Closed 4 years ago

Disable GTK3 in Firefox 42 Beta

Categories

(Core :: Widget: Gtk, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox41 --- unaffected
firefox42 --- disabled
firefox43 --- wontfix
firefox44 --- wontfix

People

(Reporter: jrmuizel, Assigned: jrmuizel)

References

Details

Attachments

(1 file)

I want to buy us some more time (another release) to iron out some of the GTK3 bugs. When I tested this on try the test coverage seemed fine.
Approval Request Comment
[Feature/regressing bug #]: Enabling GTK3
[User impact if declined]: There are various regressions that we'd like another release to address before shipping GTK3
[Describe test coverage new/current, TreeHerder]: GTK2 hasn't been tested since GTK3 was turned on but hopefully not very much has broken
[Risks and why]: Should be low risk because it's just disabling a new feature
Attachment #8664825 - Flags: review?(karlt)
Attachment #8664825 - Flags: approval-mozilla-beta?
Comment on attachment 8664825 [details] [diff] [review]
Disable GTK3 on Beta

Doing this makes sense to me but better get Mike to confirm this does what we want.

I wonder what additional changes are required to get build/unix/mozconfig.gtk to DTRT?
Attachment #8664825 - Flags: review?(mh+mozilla)
Attachment #8664825 - Flags: review?(karlt)
Attachment #8664825 - Flags: review+
Comment on attachment 8664825 [details] [diff] [review]
Disable GTK3 on Beta

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

Please also remove the gtk3.tar.xz entries in the various tooltool manifests. Otherwise, you're still building with gtk3 on automation, because mozconfig.gtk adds an explicit --enable-default-toolkit=cairo-gtk3.
Attachment #8664825 - Flags: review?(mh+mozilla) → feedback+
Comment on attachment 8664825 [details] [diff] [review]
Disable GTK3 on Beta

ok, make sense. I will remove that from the release notes.
Attachment #8664825 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8664825 [details] [diff] [review]
Disable GTK3 on Beta

Reset the uplift because it seems it needs other steps.
Attachment #8664825 - Flags: approval-mozilla-beta+ → approval-mozilla-beta?
I've already done the other steps locally.
Comment on attachment 8664825 [details] [diff] [review]
Disable GTK3 on Beta

OK, thanks
Attachment #8664825 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1208992
this is showing a large set of regressions on mozilla-beta:
http://alertmanager.allizom.org:8080/alerts.html?rev=2ad882095986&showAll=1&testIndex=0&platIndex=0

from reading this bug and looking at history, I would say these are expected, please confirm.
Flags: needinfo?(jmuizelaar)
The change here is expected to revert the changes tracked bug 1186229, but there are no performance changes recorded there.
Actually bug 1186003 is the one that would have triggered perf changes.
tscroll-ASAP		-25.8%	- returns to pre-merge beta values
TP5 Scroll		-28%	- returns to pre-merge beta values
SVG-ASAP	Ubuntu 		-10.5%	- returns to pre-merge beta values
Tab Animation Test	+2.92%	- not a regression
Talos Page Switch	-22.3%	- wasn't measured for beta before merge
Tp5 Optimized		-3.42%	- returns to pre-merge beta values
Ts, Paint		+8.38%	- not a regression

In conclusion, I don't think there's anything to worry about here.
Flags: needinfo?(jmuizelaar)
[clarifying summary to mention version number instead of just the general "beta".]
Summary: Disable GTK3 on Beta → Disable GTK3 in Firefox 42 Beta
Disabled in 42 beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/2ad882095986
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Depends on: 1211880
Is this supposed to be enabled or disabled in firefox 43?  It appears to be enabled based on a talos graph of ts_paint:
http://graphs.mozilla.org/graph.html#tests=[[83,53,35],[83,52,35],[83,64,35]]&sel=1440366804207.4285,1446498735636,788.7254602768842,1141.6666367474722&displayrange=90&datatype=geo
Flags: needinfo?(sledru)
It's not surprising that it appears to be enabled; it'd require a targeted patch to disable it, like the one we landed here for 42 beta.  (Also note that this bug is marked "status-firefox43:wontfix", as in "won't disable").

But, there do seem to be some remaining open bugs that block bug 1193807 (which is "Tracking bug for blockers to GTK3 shipping to the user").  So maybe that means we should disable this on beta again, until that bug's blocker-free & its blockers' patches have been backported as-needed?

Adding ni=karlt since he's been managing blockers on bug 1193807 most recently.
Flags: needinfo?(karlt)
I'm going to look into a solution for bug 726483.
If we can uplift a fix for that and other bugs that have already been fixed on m-c, then I think we can plan to ship the GTK3 port for 43.

There are still some appearance imperfections, but I don't know of any major usability bugs.

GTK is a moving target so we're never going to be perfect, but by getting the GTK3 port out there, there's a chance that it will be tested against prospective theme and GTK changes, and we may have less to fix.
Flags: needinfo?(karlt)
Joel, it is supposed to be enabled in 43. And I agree with Karl about 43.
Flags: needinfo?(sledru)
Sounds good to me, Karl, just let me know when you have that work ready.
You need to log in before you can comment on or make changes to this bug.