Closed
Bug 1430483
Opened 6 years ago
Closed 6 years ago
Remove remaining gtk2 and qt build options from Thunderbird and SeaMonkey
Categories
(MailNews Core :: Build Config, enhancement)
MailNews Core
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 60.0
People
(Reporter: frg, Assigned: frg)
Details
Attachments
(3 files, 1 obsolete file)
4.94 KB,
patch
|
ewong
:
review+
|
Details | Diff | Splinter Review |
14.15 KB,
patch
|
tomprince
:
review+
|
Details | Diff | Splinter Review |
2.56 KB,
patch
|
jorgk-bmo
:
review+
tomprince
:
review+
|
Details | Diff | Splinter Review |
It is no longer possible to build Thunderbird or SeaMonkey with gtk2 or qt. Only gtk3 is supported with *nix and qt was already removed some time ago. There are still references for both in the build and make files.
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #8942524 -
Flags: review?(mozilla)
Assignee | ||
Comment 2•6 years ago
|
||
First reviewer of this patch wins :)
Attachment #8942525 -
Flags: review?(iann_bugzilla)
Attachment #8942525 -
Flags: review?(ewong)
Comment 3•6 years ago
|
||
Comment on attachment 8942525 [details] [diff] [review] 1430483-mozwidget-sm.patch Review of attachment 8942525 [details] [diff] [review]: ----------------------------------------------------------------- r+ w/ nit fixed. ::: suite/installer/package-manifest.in @@ +141,5 @@ > #ifdef MOZ_FFVPX > @BINPATH@/@DLL_PREFIX@mozavutil@DLL_SUFFIX@ > @BINPATH@/@DLL_PREFIX@mozavcodec@DLL_SUFFIX@ > #endif > +#ifdef MOZ_GTK as I understand this, this should retain the MOZ_GTK3.
Attachment #8942525 -
Flags: review?(ewong) → review+
Assignee | ||
Comment 4•6 years ago
|
||
> as I understand this, this should retain the MOZ_GTK3. No longer needed. This was previously set in suite/installer/Makefile.in and could now be simplified to only set gtk. Previously there was gtk2 and gtk3 which is no longer the case. I didn't find any other cases where this was used in the installer. https://dxr.mozilla.org/comm-central/search?q=MOZ_GTK3&redirect=false > ifneq (,$(filter gtk%,$(MOZ_WIDGET_TOOLKIT))) > DEFINES += -DMOZ_GTK=1 > ifeq ($(MOZ_WIDGET_TOOLKIT),gtk3) > DEFINES += -DMOZ_GTK3=1 Fx still retains it for now but it is imho a useless differentiation. gtk is gtk3 now. Lets see if Tom agrees for TB.
Comment 5•6 years ago
|
||
Comment on attachment 8942524 [details] [diff] [review] 1430483-mozwidget-tb.patch Review of attachment 8942524 [details] [diff] [review]: ----------------------------------------------------------------- We should verify whether the bellow comment about drag and drop is still true, or if it is just something that we have carried forward long past when the original issue is fixed upstream. Other than that, this looks good. ::: mailnews/mailnews.js @@ +857,5 @@ > // Work around bug 482811 by disabling slow script warning for chrome scripts on Mac > pref("dom.max_chrome_script_run_time", 0); > #endif > > +// gtk3 (*nix) lacks transparent/translucent drag support (bug 376238), so we Is this actually true? It looks like firefox enables this unconditionally now, so I think the bug this is working around has actually been fixed, so we can remove this setting (both here and in `mail/app/profile/all-thunderbird.js`.
Attachment #8942524 -
Flags: review?(mozilla) → feedback+
Assignee | ||
Comment 6•6 years ago
|
||
The option is still in the source and might be needed for non compositing windows managers: https://en.wikipedia.org/wiki/Compositing_window_manager I also missed that it is duplicated in all-thunderbird.js: https://dxr.mozilla.org/comm-central/search?q=nglayout.enable_drag_images&redirect=false I would rather only deal with the comments here and open a followup bug in case we remove the options and it turns out that it needs to stay in and be backed out. Or I split the patch in 2 parts.
Assignee | ||
Comment 7•6 years ago
|
||
rebased patch
Attachment #8942524 -
Attachment is obsolete: true
Attachment #8943200 -
Flags: review?(mozilla)
Assignee | ||
Comment 8•6 years ago
|
||
Additional patch removing the option from the pref files. This probably needs to be tested first.
Attachment #8943201 -
Flags: feedback?(mozilla)
Attachment #8943201 -
Flags: feedback?(jorgk)
Comment 9•6 years ago
|
||
Comment on attachment 8943201 [details] [diff] [review] 1430483-mozwidget-tb-part2.patch The comment refers to bug 376238 which was fixed in two changesets: https://hg.mozilla.org/mozilla-central/rev/4369a4cf2b0a https://hg.mozilla.org/mozilla-central/rev/a59b394a47aa So as far as I can see, dragging images on Linux works in some way, so I think we can lose the code as proposed here. The pref is still at that value today: https://dxr.mozilla.org/mozilla-central/rev/04c0a07b8de21300856ec89b7d118d4be9b86250/modules/libpref/init/all.js#1134
Attachment #8943201 -
Flags: feedback?(jorgk) → review+
Assignee | ||
Comment 10•6 years ago
|
||
Comment on attachment 8942525 [details] [diff] [review] 1430483-mozwidget-sm.patch As per addtional irc comments ewong is ok with removing MOZ_GTK3 and only using MOZ_GTK in the installer package. Patch needs a minor rebase for new MOZ_WAYLAND there which I will do when checking it in.
Attachment #8942525 -
Flags: review?(iann_bugzilla)
Comment 11•6 years ago
|
||
Comment on attachment 8943201 [details] [diff] [review] 1430483-mozwidget-tb-part2.patch Review of attachment 8943201 [details] [diff] [review]: ----------------------------------------------------------------- This should land before the previous patch, but otherwise it looks good.
Attachment #8943201 -
Flags: feedback?(mozilla) → review+
Updated•6 years ago
|
Attachment #8943200 -
Flags: review?(mozilla) → review+
Assignee | ||
Comment 12•6 years ago
|
||
Thanks. Jorg, do you want to land all three?
Flags: needinfo?(jorgk)
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 14•6 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/b866cf8e5eb4 Remove no longer valid gtk2 build options from SeaMonkey. r=ewong https://hg.mozilla.org/comm-central/rev/d0d84ec8115f Remove no longer valid gtk2 and qt build options from Thunderbird. r=tomprince https://hg.mozilla.org/comm-central/rev/fdc55627daf8 Remove no longer needed gtk pref nglayout.enable_drag_images from Thunderbird. r=tomprince,jorgk
Updated•6 years ago
|
Target Milestone: --- → Thunderbird 60.0
You need to log in
before you can comment on or make changes to this bug.
Description
•