Remove the GTK2 code (MOZ_WIDGET_GTK == 2)

RESOLVED FIXED in Firefox 59

Status

()

defect
P5
normal
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: jwatt, Assigned: sylvestre)

Tracking

unspecified
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: tpi:+)

Attachments

(6 attachments, 5 obsolete attachments)

2.19 KB, patch
glandium
: review+
Details | Diff | Splinter Review
59 bytes, text/x-review-board-request
lsalzman
: review+
Details
59 bytes, text/x-review-board-request
lsalzman
: review+
karlt
: review+
Details
59 bytes, text/x-review-board-request
lsalzman
: review+
Details
59 bytes, text/x-review-board-request
lsalzman
: review+
Details
59 bytes, text/x-review-board-request
lsalzman
: review+
Details
(Reporter)

Description

3 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.
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.
Yes, the Gtk2 is widely utilized by Enterprise distros like RHEL and SLES so please leave it here.
Priority: -- → P5
Whiteboard: tpi:+
Red Hat can go along with the gtk3 only Firefox when it comes in next ESR cycle (FF59).
(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?
Flags: needinfo?(stransky)
(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.
Flags: needinfo?(stransky)
Mike, any issues on the Debian/Ubuntu end, or would we be clear there as well?
Flags: needinfo?(mh+mozilla)
(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.
And Wolfgang from SUSE?
Flags: needinfo?(mozilla)
As far as Debian is concerned, this works.
Flags: needinfo?(mh+mozilla)
Or maybe Peter is the right person from Suse.
Flags: needinfo?(pcerny)
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.
Attachment #8823398 - Flags: review?(mh+mozilla)
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.
Attachment #8823398 - Flags: review?(mh+mozilla)
v2... Fixed line wrapping and tuple.
Attachment #8823398 - Attachment is obsolete: true
Attachment #8825648 - Flags: review?(mh+mozilla)
Attachment #8825648 - Flags: review?(mh+mozilla) → review+
(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.
Flags: needinfo?(pcerny)

Comment 15

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

Comment 17

2 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?
(Assignee)

Updated

2 years ago
Assignee: nobody → lsalzman
(Assignee)

Comment 18

2 years ago
I guess because no one closed it :)
I just did

Comment 19

2 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.
Blocks: 1343802, 1306329
(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.
(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?
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.
See Also: → 1384701
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.
(Reporter)

Comment 24

a year ago
Any chance you can finish that off now, or do you need someone else to do that?
Flags: needinfo?(lsalzman)
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.
Assignee: lsalzman → nobody
Flags: needinfo?(lsalzman)
(Assignee)

Comment 26

a year ago
I will give it a try. I am removing other codes
Assignee: nobody → sledru
(Assignee)

Comment 27

a year ago
I made some progress on that.

Just a question, should I remove dom/plugins/test/testplugin/nptest_gtk2.cpp ?
(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.
(Assignee)

Comment 29

a year ago
Ok, that means that we should keep the configure detection then.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 39

a year 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)

Comment 40

a year 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.
Attachment #8938416 - Flags: review?(lsalzman) → review-
(Assignee)

Comment 41

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

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

Comment 42

a year ago
mozreview-review
Comment on attachment 8938420 [details]
Bug 1278282 - Ride along Remove some trailing whitespaces

https://reviewboard.mozilla.org/r/209120/#review214942
Attachment #8938420 - Flags: review?(lsalzman) → review+

Comment 43

a year ago
mozreview-review
Comment on attachment 8938421 [details]
Bug 1278282 - remove old and commented mozconfig options

https://reviewboard.mozilla.org/r/209122/#review214944
Attachment #8938421 - Flags: review?(lsalzman) → review+

Comment 44

a year 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
Attachment #8938422 - Flags: review?(lsalzman) → review+

Comment 45

a year ago
mozreview-review
Comment on attachment 8938423 [details]
Bug 1278282 - Remove some gtk2 options in the configure

https://reviewboard.mozilla.org/r/209126/#review214948
Attachment #8938423 - Flags: review?(lsalzman) → review+

Comment 46

a year 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.

Comment 47

a year 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
Attachment #8938417 - Flags: review?(lsalzman) → review+

Comment 48

a year 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
Attachment #8938419 - Flags: review?(lsalzman) → review+
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.
(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.
Flags: needinfo?(karlt)
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.

Comment 52

a year 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.
(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
Flags: needinfo?(karlt)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8938416 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Attachment #8938421 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Attachment #8938424 - Attachment is obsolete: true
Attachment #8938424 - Flags: review?(lsalzman)

Comment 60

a year 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.
Attachment #8938417 - Flags: review-

Comment 61

a year 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.

Comment 62

a year 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.
Attachment #8938418 - Flags: review?(karlt) → review+
Attachment #8938418 - Flags: review?(lsalzman) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8938423 - Attachment is obsolete: true

Comment 68

a year 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'>
Attachment #8938417 - Flags: review-
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 74

a year ago
mozreview-review
Comment on attachment 8938418 [details]
Bug 1278282 - Remove the 'MOZ_WIDGET_GTK == 2' defines

https://reviewboard.mozilla.org/r/209116/#review217618
Attachment #8938418 - Flags: review?(lsalzman) → review+

Comment 75

a year 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
(Assignee)

Updated

a year ago
Keywords: leave-open
Flags: needinfo?(mozilla)
You need to log in before you can comment on or make changes to this bug.