Jonathan Watt [:jwatt] Reporter
	Description • 8 years ago

Once we're happy that the GTK3 port (bug 627699) is sufficiently stable and we're not going back to re-enabling GTK2 we should remove the code that is behind the |MOZ_WIDGET_GTK == 2| checks.

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

I don't think we'll be ready to remove this soon.
I expect Redhat are still using it?
And there is good reason for distributions to be using it due to GTK3 not being a stable platform.

I consider it a tier 2 port.  I don't compile or test my patches against GTK2 but I aim to write code that should work for both GTK 2 and 3.

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

Yes, the Gtk2 is widely utilized by Enterprise distros like RHEL and SLES so please leave it here.

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

Red Hat can go along with the gtk3 only Firefox when it comes in next ESR cycle (FF59).

	Lee Salzman [:lsalzman]
	Comment 4 • 8 years ago

(In reply to Martin Stránský from comment #3)
> Red Hat can go along with the gtk3 only Firefox when it comes in next ESR
> cycle (FF59).

To be clear, Red Hat is shipping 52 ESR, that means starting with 53, we would be safe to remove gtk2 support?

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

(In reply to Lee Salzman [:lsalzman] from comment #4)
> (In reply to Martin Stránský from comment #3)
> > Red Hat can go along with the gtk3 only Firefox when it comes in next ESR
> > cycle (FF59).
> 
> To be clear, Red Hat is shipping 52 ESR, that means starting with 53, we
> would be safe to remove gtk2 support?

Yes, we'd definitely need to ship 52 ESR as gtk2, but we can ship 59 ESR as gtk3 only.

	Lee Salzman [:lsalzman]
	Comment 6 • 8 years ago

Mike, any issues on the Debian/Ubuntu end, or would we be clear there as well?

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

(In reply to Lee Salzman [:lsalzman] from comment #4)
> (In reply to Martin Stránský from comment #3)
> > Red Hat can go along with the gtk3 only Firefox when it comes in next ESR
> > cycle (FF59).
> 
> To be clear, Red Hat is shipping 52 ESR, that means starting with 53, we
> would be safe to remove gtk2 support?

And yes, we ship only ESR lines in RHEL so remove that in 53 is fine for us.

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

And Wolfgang from SUSE?

	Mike Hommey [:glandium]
	Comment 9 • 8 years ago

As far as Debian is concerned, this works.

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

Or maybe Peter is the right person from Suse.

	Lee Salzman [:lsalzman]
	Comment 11 • 8 years ago

After discussing with Jeff, we are going to forge ahead with disabling the option to build with GTK2 as a tentative step aimed at generating more feedback, since so far we have failed to generate sufficient negative feedback to derail this. If someone has a problem with the lack of GTK2 build options, this will better allow us to figure that out.

Further removal of all the GTK2 cruft can follow once we're sure we wouldn't want to back this out.

	Mike Hommey [:glandium]
	Comment 12 • 7 years ago

Comment on attachment 8823398 [details] [diff] [review]
disallow building with gtk2 toolkit

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

::: toolkit/moz.configure
@@ +116,5 @@
>  # config.guess, which we want to avoid. Even better, we could actually set
>  # `choices` depending on the target, but that doesn't pan out for the same
>  # reason.
>  option('--enable-default-toolkit', nargs=1,
> +       choices=('cairo-windows', 'cairo-gtk3',

Please rewrap the lines.

@@ +140,5 @@
>              platform_choices = tuple(value)
>          else:
>              platform_choices = ('cairo-android',)
>      else:
> +        platform_choices = ('cairo-gtk3')

You need a comma after 'cairo-gtk3', like the others, otherwise, python doesn't make it a tuple, and it needs to be a tuple for the things below.

	Lee Salzman [:lsalzman]
	Comment 13 • 7 years ago

v2... Fixed line wrapping and tuple.

	Petr Cerny [:hrosik]
	Comment 14 • 7 years ago

(In reply to Martin Stránský from comment #10)
> Or maybe Peter is the right person from Suse.

SLE can also go with GTK3 for FF 59 ESR (so removal in FF 53 is ok). As for openSUSE, I think it should work out of the box, unless GTK3 newer than 3.16 is required.

	Pulsebot
	Comment 15 • 7 years ago

Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/529160328d96
disallow building with gtk2 toolkit. r=glandium

	Carsten Book [:Tomcat]
	Comment 16 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/529160328d96

	Ian Neal
	Comment 17 • 7 years ago

(In reply to Jonathan Watt [:jwatt] from comment #0)
> Once we're happy that the GTK3 port (bug 627699) is sufficiently stable and
> we're not going back to re-enabling GTK2 we should remove the code that is
> behind the |MOZ_WIDGET_GTK == 2| checks.

As bug 627699 is still open, just wondering why this landed?

	Sylvestre Ledru [:Sylvestre] Assignee
	Comment 18 • 7 years ago

I guess because no one closed it :)
I just did

	Jan Beich
	Comment 19 • 7 years ago

Even if one wants to keep using Gtk2 downstream bitrot gets in the way:

- mozilla-central changeset a5aa102e8f0f crashes:

  (lldb) bt
  * thread #1, stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
    * frame #0: 0x0000000000000000
      frame #1: firefox`mozilla::GetBootstrap(char const*) at nsXPCOMGlue.cpp:372
      frame #2: firefox`mozilla::GetBootstrap(aXPCOMFile=<unavailable>) at nsXPCOMGlue.cpp:418
      frame #3: firefox`InitXPCOMGlue(argv0=<unavailable>) at nsBrowserApp.cpp:246
      frame #4: firefox`main(argc=4, argv=0x00007fffffffe1b8, envp=0x00007fffffffe1e0) at nsBrowserApp.cpp:294
      frame #5: firefox`_start(ap=<unavailable>, cleanup=<unavailable>) at crt1.c:72
  (lldb) f 1
  frame #1: firefox`mozilla::GetBootstrap(char const*) at nsXPCOMGlue.cpp:372
     369          "XRE_GlibInit");
     370        // Initialize glib enough for G_SLICE to have an effect before it is unset.
     371        // unset.
  -> 372        XRE_GlibInit();
     373      }
     374  #endif
     375      if (!mHadGSlice) {

- mozilla-central changeset 8c45140a2819 broke build:

  In file included from objdir/widget/gtk/Unified_c_widget_gtk0.c:2:
  In file included from widget/gtk/gtk2drawing.c:14:
  widget/gtk/gtkdrawing.h:49:3: error: must use 'struct' tag to refer to type 'MozGtkSize'
    MozGtkSize& operator+=(const GtkBorder& aBorder)
    ^
    struct
  widget/gtk/gtkdrawing.h:49:13: error: expected member name or ';' after declaration specifiers
    MozGtkSize& operator+=(const GtkBorder& aBorder)
	      ^
  widget/gtk/gtkdrawing.h:49:3: error: field has incomplete type 'struct MozGtkSize'
    MozGtkSize& operator+=(const GtkBorder& aBorder)
    ^
  widget/gtk/gtkdrawing.h:45:8: note: definition of 'struct MozGtkSize' is not complete until the closing '}'
  struct MozGtkSize {
	 ^
  widget/gtk/gtkdrawing.h:49:13: error: expected ';' at end of declaration list
    MozGtkSize& operator+=(const GtkBorder& aBorder)
	      ^
	      ;
  widget/gtk/gtkdrawing.h:74:3: error: unknown type name 'bool'
    bool initialized;
    ^
  widget/gtk/gtkdrawing.h:76:5: error: must use 'struct' tag to refer to type 'MozGtkSize'
      MozGtkSize scrollbar;
      ^
      struct
  widget/gtk/gtkdrawing.h:77:5: error: must use 'struct' tag to refer to type 'MozGtkSize'
      MozGtkSize thumb;
      ^
      struct
  widget/gtk/gtkdrawing.h:78:5: error: must use 'struct' tag to refer to type 'MozGtkSize'
      MozGtkSize button;
      ^
      struct
  In file included from objdir/widget/gtk/Unified_c_widget_gtk0.c:2:
  widget/gtk/gtk2drawing.c:3167:31: error: unknown type name 'MozGtkScrollbarMetrics'
  moz_gtk_get_scrollbar_metrics(MozGtkScrollbarMetrics *metrics)
				^
  9 errors generated.

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

(In reply to Jan Beich from comment #19)
> Even if one wants to keep using Gtk2 downstream bitrot gets in the way:
> - mozilla-central changeset 8c45140a2819 broke build:
> 
>   In file included from objdir/widget/gtk/Unified_c_widget_gtk0.c:2:
>   In file included from widget/gtk/gtk2drawing.c:14:
>   widget/gtk/gtkdrawing.h:49:3: error: must use 'struct' tag to refer to
> type 'MozGtkSize'
>     MozGtkSize& operator+=(const GtkBorder& aBorder)
>     ^
>     struct
>   widget/gtk/gtkdrawing.h:49:13: error: expected member name or ';' after
> declaration specifiers
>     MozGtkSize& operator+=(const GtkBorder& aBorder)
> 	      ^
>   widget/gtk/gtkdrawing.h:49:3: error: field has incomplete type 'struct
> MozGtkSize'
>     MozGtkSize& operator+=(const GtkBorder& aBorder)
>     ^
>   widget/gtk/gtkdrawing.h:45:8: note: definition of 'struct MozGtkSize' is
> not complete until the closing '}'
>   struct MozGtkSize {
> 	 ^
>   widget/gtk/gtkdrawing.h:49:13: error: expected ';' at end of declaration
> list
>     MozGtkSize& operator+=(const GtkBorder& aBorder)
> 	      ^
> 	      ;
>   widget/gtk/gtkdrawing.h:74:3: error: unknown type name 'bool'
>     bool initialized;
>     ^
>   widget/gtk/gtkdrawing.h:76:5: error: must use 'struct' tag to refer to
> type 'MozGtkSize'
>       MozGtkSize scrollbar;
>       ^
>       struct
>   widget/gtk/gtkdrawing.h:77:5: error: must use 'struct' tag to refer to
> type 'MozGtkSize'
>       MozGtkSize thumb;
>       ^
>       struct
>   widget/gtk/gtkdrawing.h:78:5: error: must use 'struct' tag to refer to
> type 'MozGtkSize'
>       MozGtkSize button;
>       ^
>       struct
>   In file included from objdir/widget/gtk/Unified_c_widget_gtk0.c:2:
>   widget/gtk/gtk2drawing.c:3167:31: error: unknown type name
> 'MozGtkScrollbarMetrics'
>   moz_gtk_get_scrollbar_metrics(MozGtkScrollbarMetrics *metrics)
> 				^
>   9 errors generated.

The MozGtkSize is used in Gtk3 only, you can simply encapsulate it by:

#if (MOZ_WIDGET_GTK == 3)
...
#endif

and use original MozGtkScrollbarMetrics for Gtk2. I can attach the patch if anyone needs that.

	Mike Hommey [:glandium]
	Comment 21 • 7 years ago

(In reply to Martin Stránský from comment #7)
> (In reply to Lee Salzman [:lsalzman] from comment #4)
> > (In reply to Martin Stránský from comment #3)
> > > Red Hat can go along with the gtk3 only Firefox when it comes in next ESR
> > > cycle (FF59).
> > 
> > To be clear, Red Hat is shipping 52 ESR, that means starting with 53, we
> > would be safe to remove gtk2 support?
> 
> And yes, we ship only ESR lines in RHEL so remove that in 53 is fine for us.

Don't you need it when 52ESR is EOLed and 59 is the ESR?

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

Trying to keep the GTK2 port going is not recommended because the client-side graphics rendering will give poor performance when natively themed widgets are read back from the server.

Perhaps Flatpak may be a way to support GTK3 apps on older systems.

I removed the GTK2 code from nsLookAndFeel for bug 1384701 because it was in the way and no longer used AIUI.

	Lee Salzman [:lsalzman]
	Comment 23 • 7 years ago

We already established the consensus that 52/ESR was the last release we were going to truly support it. Beyond that, we were/are free to just break/remove anything related to GTK2 we wish. I had a giant patch to just remove all the GTK2 code several months back, but in the run up to 57 I have been too busy to finish it.

	Jonathan Watt [:jwatt] Reporter
	Comment 24 • 7 years ago

Any chance you can finish that off now, or do you need someone else to do that?

	Lee Salzman [:lsalzman]
	Comment 25 • 7 years ago

Too busy right now to finish this. So if someone else can in short order, that would be great. In any case, any GTK2 code remaining in our tree is to be considered dead anyway, and there is no need to fix it when you're working nearby.

	Sylvestre Ledru [:Sylvestre] Assignee
	Comment 26 • 7 years ago

I will give it a try. I am removing other codes

	Sylvestre Ledru [:Sylvestre] Assignee
	Comment 27 • 7 years ago

I made some progress on that.

Just a question, should I remove dom/plugins/test/testplugin/nptest_gtk2.cpp ?

	Lee Salzman [:lsalzman]
	Comment 28 • 7 years ago

(In reply to Sylvestre Ledru [:sylvestre] from comment #27)
> I made some progress on that.
> 
> Just a question, should I remove dom/plugins/test/testplugin/nptest_gtk2.cpp
> ?

Plugins like Flash can still legitimately be GTK2. It is just our internal widget code that no longer is.

	Sylvestre Ledru [:Sylvestre] Assignee
	Comment 29 • 7 years ago

Ok, that means that we should keep the configure detection then.

	Sylvestre Ledru [:Sylvestre] Assignee
	Comment 39 • 7 years ago

Sorry Lee if you are not the right reviewer. As you were going to work on the removal, I assumed that you were the right person.

I run a try job (before merging all patches), it is green https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fd114f294a81763471318f9abb6687742051331
(l10n isn't my fault and is tier-2)

	Lee Salzman [:lsalzman]
	Comment 40 • 7 years ago
mozreview-review

Comment on attachment 8938416 [details]
Bug 1278282 - Remove gtk2drawing.c & the gtk2 directory

https://reviewboard.mozilla.org/r/209112/#review214930

We still need mozgtk to load the GTK2 flash plugin.

	Sylvestre Ledru [:Sylvestre] Assignee
	Comment 41 • 7 years ago

> We still need mozgtk to load the GTK2 flash plugin.

OK, thanks. Do you know if we have any testing covering that?

	Lee Salzman [:lsalzman]
	Comment 42 • 7 years ago
mozreview-review

Comment on attachment 8938420 [details]
Bug 1278282 - Ride along Remove some trailing whitespaces

https://reviewboard.mozilla.org/r/209120/#review214942

	Lee Salzman [:lsalzman]
	Comment 43 • 7 years ago
mozreview-review

Comment on attachment 8938421 [details]
Bug 1278282 - remove old and commented mozconfig options

https://reviewboard.mozilla.org/r/209122/#review214944

	Lee Salzman [:lsalzman]
	Comment 44 • 7 years ago
mozreview-review

Comment on attachment 8938422 [details]
Bug 1278282 - Update of the tests to reflect the removal of the gtk2

https://reviewboard.mozilla.org/r/209124/#review214946

	Lee Salzman [:lsalzman]
	Comment 45 • 7 years ago
mozreview-review

Comment on attachment 8938423 [details]
Bug 1278282 - Remove some gtk2 options in the configure

https://reviewboard.mozilla.org/r/209126/#review214948

	Lee Salzman [:lsalzman]
	Comment 46 • 7 years ago
mozreview-review

Comment on attachment 8938418 [details]
Bug 1278282 - Remove the 'MOZ_WIDGET_GTK == 2' defines

https://reviewboard.mozilla.org/r/209116/#review214950

I am not sure it is safe to rip out gtk2xtbin here because of the flash plugin situation.

	Lee Salzman [:lsalzman]
	Comment 47 • 7 years ago
mozreview-review

Comment on attachment 8938417 [details]
Bug 1278282 - update of the moz.build files to remove gtk2 references

https://reviewboard.mozilla.org/r/209114/#review214952

	Lee Salzman [:lsalzman]
	Comment 48 • 7 years ago
mozreview-review

Comment on attachment 8938419 [details]
Bug 1278282 - Replace #if (MOZ_WIDGET_GTK == 3) by #ifdef MOZ_WIDGET_GTK

https://reviewboard.mozilla.org/r/209118/#review214956

	Lee Salzman [:lsalzman]
	Comment 49 • 7 years ago

Overall, I am concerned that this series of patches has some stuff in it that could end up breaking GTK2 flash plugin? Can we verify this still works?

Also, we have some workarounds for buggy Cairo/XShm (bug 1271100) living in mozgtk, which we would need to find a new home for before mozgtk could be nuked as well, ignoring the flash plugin situation.

	Lee Salzman [:lsalzman]
	Comment 50 • 7 years ago

(In reply to Sylvestre Ledru [:sylvestre] from comment #41)
> > We still need mozgtk to load the GTK2 flash plugin.
> 
> OK, thanks. Do you know if we have any testing covering that?

We might have something in dom/plugins/test/testplugin, but how well or if that tests compatibility with flash plugin, and the extent to which mozgtk is required for that, I am not entirely sure. karlt would probably be better there.

	Mike Hommey [:glandium]
	Comment 51 • 7 years ago

To make thing clear, the scope of this bug is to remove everything that would be built with --enable-default-toolkit=cairo-gtk2, but isn't built with --enable-default-toolkit=cairo-gtk3.

IOW, nothing should change for --enable-default-toolkit=cairo-gtk3 builds.

	Karl Tomlinson (:karlt)
	Comment 52 • 7 years ago
mozreview-review-reply

Comment on attachment 8938418 [details]
Bug 1278282 - Remove the 'MOZ_WIDGET_GTK == 2' defines

https://reviewboard.mozilla.org/r/209116/#review214950

The "#if (MOZ_WIDGET_GTK == 2)" code can be stripped even from plugin code.
Plugin code is built with MOZ_GTK2_CFLAGS rather than changing MOZ_WIDGET_GTK

The nsNPAPIPlugin gtk2xtbin code was for in-process plugins.  It was not
ported to GTK3 and is not required.  Out of process plugins do support this
with GTK3, but Flash does not use it.  Other plugins are no longer supported
and so all gtk2xtbin code can be removed.  That is a separate issue from this
bug though, and the patches here don't remove it AFAICS.

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

(In reply to Lee Salzman [:lsalzman] from comment #50)
> (In reply to Sylvestre Ledru [:sylvestre] from comment #41)
> > > We still need mozgtk to load the GTK2 flash plugin.
> > 
> > OK, thanks. Do you know if we have any testing covering that?
> 
> We might have something in dom/plugins/test/testplugin, but how well or if
> that tests compatibility with flash plugin, and the extent to which mozgtk
> is required for that, I am not entirely sure. karlt would probably be better
> there.

Usually bad things happen when GTK2 and GTK3 libs are loaded in the same
process.  The different versions of libmozgtk.so prevent that.  Without
different libmozgtk, GTK2 and GTK3 libs should both be loaded with any of
the tests using the test plugin, but I guess the test plugin is not using
enough of GTK to trigger the problems, which surprises me.

https://searchfox.org/mozilla-central/search?q=application%2Fx-test

	Mike Hommey [:glandium]
	Comment 60 • 6 years ago
mozreview-review

Comment on attachment 8938417 [details]
Bug 1278282 - update of the moz.build files to remove gtk2 references

https://reviewboard.mozilla.org/r/209114/#review217284

::: dom/plugins/ipc/moz.build:134
(Diff revision 2)
>      CXXFLAGS += CONFIG['TK_CFLAGS']
> -else:
> -    # Force build against gtk+2 for struct offsets and such.
> -    CXXFLAGS += CONFIG['MOZ_GTK2_CFLAGS']
>  
> +CXXFLAGS += CONFIG['MOZ_GTK2_CFLAGS']

This should still be in the else case.

::: dom/plugins/test/testplugin/testplugin.mozbuild:53
(Diff revision 2)
>  
> -if CONFIG['MOZ_WIDGET_TOOLKIT'] in ('gtk2', 'gtk3'):
> +if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gtk3':
>      CXXFLAGS += CONFIG['MOZ_GTK2_CFLAGS']
> -    CFLAGS += CONFIG['MOZ_GTK2_CFLAGS']
>      OS_LIBS += CONFIG['MOZ_GTK2_LIBS']
> +    CFLAGS += CONFIG['MOZ_GTK2_CFLAGS']

it would be better to leave this next to CXXFLAGS like originally.

::: toolkit/moz.configure:149
(Diff revision 2)
>  
>  @depends(toolkit)
>  def toolkit(toolkit):
> -    if toolkit == 'cairo-gtk2-x11':
> -        widget_toolkit = 'gtk2'
>      elif toolkit == 'cairo-gtk3-wayland' :

you need to replace elif with if.

::: toolkit/moz.configure:180
(Diff revision 2)
>  @depends('--without-x', toolkit)
>  def x11(value, toolkit):
>      if not value:
>          die('--without-x is not supported')
>  
> -    x11_toolkits = ('gtk2', 'gtk3')
> +    x11_toolkits = 'gtk3'

The tests that follow want this to be a tuple. Either make it a tuple or change the tests.

::: widget/gtkxtbin/moz.build:23
(Diff revision 2)
>  FINAL_LIBRARY = 'xul'
>  
>  DEFINES['_IMPL_GTKXTBIN_API'] = True
>  
>  CFLAGS += CONFIG['MOZ_GTK2_CFLAGS']
> +

unnecessary change.

	Mike Hommey [:glandium]
	Comment 61 • 6 years ago
mozreview-review

Comment on attachment 8938422 [details]
Bug 1278282 - Update of the tests to reflect the removal of the gtk2

https://reviewboard.mozilla.org/r/209124/#review217290

::: dom/plugins/test/mochitest/mochitest.ini
(Diff revision 2)
> -[test_plugin_scroll_invalidation.html]
> -skip-if = toolkit != "gtk2"
> -support-files = plugin_scroll_invalidation.html

You could remove those files too.

	Karl Tomlinson (:karlt)
	Comment 62 • 6 years ago
mozreview-review

Comment on attachment 8938418 [details]
Bug 1278282 - Remove the 'MOZ_WIDGET_GTK == 2' defines

https://reviewboard.mozilla.org/r/209116/#review217350

This all looks good to me, thanks very much.
I don't think you need to wait for Lee's review also.

::: dom/plugins/base/nsPluginsDirUnix.cpp:91
(Diff revision 2)
>  
> -#if (MOZ_WIDGET_GTK == 2)
>  

Could remove the second blank line here.

::: gfx/thebes/gfxGdkNativeRenderer.h:65
(Diff revision 2)
>  
> +

Extra blank line added here.

::: widget/gtk/IMContextWrapper.cpp:154
(Diff revision 2)
>                       NS_GET_A(aColor));
>      }
>      virtual ~GetTextRangeStyleText() {};
>  };
>  
> -const static bool kUseSimpleContextDefault = MOZ_WIDGET_GTK == 2;
> +    const static bool kUseSimpleContextDefault = false;

Indentation.

::: widget/gtk/gtkdrawing.h
(Diff revision 2)
> - * Retrieves the colormap to use for drawables passed to moz_gtk_widget_paint.
> - */
> -GdkColormap* moz_gtk_widget_get_colormap();
> -#endif
> -
> -/*** Widget drawing ***/

Could keep this comment, but I don't really mind.

	Mike Hommey [:glandium]
	Comment 68 • 6 years ago
mozreview-review

Comment on attachment 8938417 [details]
Bug 1278282 - update of the moz.build files to remove gtk2 references

https://reviewboard.mozilla.org/r/209114/#review217422

::: toolkit/moz.configure:180
(Diff revision 3)
>  @depends('--without-x', toolkit)
>  def x11(value, toolkit):
>      if not value:
>          die('--without-x is not supported')
>  
> -    x11_toolkits = ('gtk2', 'gtk3')
> +    x11_toolkits = ('gtk3')

>>> a = ('foo')
>>> type(a)
<type 'str'>
>>> a = ('foo',)
>>> type(a)
<type 'tuple'>

	Lee Salzman [:lsalzman]
	Comment 74 • 6 years ago
mozreview-review

Comment on attachment 8938418 [details]
Bug 1278282 - Remove the 'MOZ_WIDGET_GTK == 2' defines

https://reviewboard.mozilla.org/r/209116/#review217618

	Pulsebot
	Comment 75 • 6 years ago

Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0054a15e3c89
update of the moz.build files to remove gtk2 references r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/07457a9823d3
Replace #if (MOZ_WIDGET_GTK == 3) by #ifdef MOZ_WIDGET_GTK r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/64a1d5998ef0
Ride along Remove some trailing whitespaces r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/28340f2e2631
Update of the tests to reflect the removal of the gtk2 r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/26fa4e61b9bc
Remove the 'MOZ_WIDGET_GTK == 2' defines r=karlt,lsalzman

	Noemi Erli[:noemi_erli]
	Comment 76 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/0054a15e3c89
https://hg.mozilla.org/mozilla-central/rev/07457a9823d3
https://hg.mozilla.org/mozilla-central/rev/64a1d5998ef0
https://hg.mozilla.org/mozilla-central/rev/28340f2e2631
https://hg.mozilla.org/mozilla-central/rev/26fa4e61b9bc